From 63e653a21f59a17810d597ec35b20fb13bae6692 Mon Sep 17 00:00:00 2001 From: Lakshman Krishnamoorthy Date: Thu, 30 May 2019 14:56:55 -0700 Subject: [PATCH] lib: crash when FRR hostname length > 80 chars Although the RFC states hostname length should be < 255 chars, FRR allows infinite length technically. However, when you try to set a hostname > 80 chars, you would immediately notice a crash. RCA: Crash due to buffer overflow. Large buffer sprintf'd into smaller buffer. Usage of sprintf function instead of snprintf which is safer. Signed-off-by: Lakshman Krishnamoorthy --- lib/command.c | 10 +++++++++- lib/command.h | 11 +++++++++++ vtysh/vtysh.c | 2 +- vtysh/vtysh_config.c | 4 ++-- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lib/command.c b/lib/command.c index 29f41a712c..1d79dbddd2 100644 --- a/lib/command.c +++ b/lib/command.c @@ -1962,7 +1962,15 @@ DEFUN (config_hostname, struct cmd_token *word = argv[1]; if (!isalnum((int)word->arg[0])) { - vty_out(vty, "Please specify string starting with alphabet\n"); + vty_out(vty, + "Please specify string starting with alphabet or number\n"); + return CMD_WARNING_CONFIG_FAILED; + } + + /* With reference to RFC 1123 Section 2.1 */ + if (strlen(word->arg) > HOSTNAME_LEN) { + vty_out(vty, "Hostname length should be less than %d chars\n", + HOSTNAME_LEN); return CMD_WARNING_CONFIG_FAILED; } diff --git a/lib/command.h b/lib/command.h index d96ec97e67..d6c41e0824 100644 --- a/lib/command.h +++ b/lib/command.h @@ -37,6 +37,17 @@ extern "C" { DECLARE_MTYPE(HOST) DECLARE_MTYPE(COMPLETION) +/* + * From RFC 1123 (Requirements for Internet Hosts), Section 2.1 on hostnames: + * One aspect of host name syntax is hereby changed: the restriction on + * the first character is relaxed to allow either a letter or a digit. + * Host software MUST support this more liberal syntax. + * + * Host software MUST handle host names of up to 63 characters and + * SHOULD handle host names of up to 255 characters. + */ +#define HOSTNAME_LEN 255 + /* Host configuration variable */ struct host { /* Host name of this router. */ diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 51d5e42915..54dcde893e 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -3429,7 +3429,7 @@ void vtysh_readline_init(void) char *vtysh_prompt(void) { - static char buf[100]; + static char buf[512]; snprintf(buf, sizeof buf, cmd_prompt(vty->node), cmd_hostname_get()); return buf; diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c index cf94ab643a..9c2de0f62b 100644 --- a/vtysh/vtysh_config.c +++ b/vtysh/vtysh_config.c @@ -521,10 +521,10 @@ int vtysh_read_config(const char *config_default_dir) */ void vtysh_config_write(void) { - char line[81]; + char line[512]; if (cmd_hostname_get()) { - sprintf(line, "hostname %s", cmd_hostname_get()); + snprintf(line, sizeof(line), "hostname %s", cmd_hostname_get()); vtysh_config_parse_line(NULL, line); } -- 2.39.5