From 134cfa5cdae80a726ed5d444d5d8d3d1f9d27d21 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Thu, 20 Feb 2014 22:53:04 -0500 Subject: [PATCH] Fix: scanf unbounded input Awaiting a proper cleanup (introducing nscanf), do what we can to validate the scanf input using its utterly broken API. Signed-off-by: Mathieu Desnoyers Signed-off-by: David Goulet --- src/bin/lttng/commands/enable_events.c | 13 +++++++--- src/bin/lttng/conf.c | 7 +++++- src/lib/lttng-ctl/filter/filter-parser.y | 31 ++++++++++++++++++------ 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/bin/lttng/commands/enable_events.c b/src/bin/lttng/commands/enable_events.c index 29a399e97..bd2d99785 100644 --- a/src/bin/lttng/commands/enable_events.c +++ b/src/bin/lttng/commands/enable_events.c @@ -30,6 +30,10 @@ #include "../command.h" #include +#if (LTTNG_SYMBOL_NAME_LEN == 256) +#define LTTNG_SYMBOL_NAME_LEN_SCANF_IS_A_BROKEN_API "255" +#endif + static char *opt_event_list; static int opt_event_type; static const char *opt_loglevel; @@ -226,6 +230,7 @@ static int parse_probe_opts(struct lttng_event *ev, char *opt) { int ret; char s_hex[19]; +#define S_HEX_LEN_SCANF_IS_A_BROKEN_API "18" /* 18 is (19 - 1) (\0 is extra) */ char name[LTTNG_SYMBOL_NAME_LEN]; if (opt == NULL) { @@ -234,7 +239,8 @@ static int parse_probe_opts(struct lttng_event *ev, char *opt) } /* Check for symbol+offset */ - ret = sscanf(opt, "%[^'+']+%s", name, s_hex); + ret = sscanf(opt, "%" LTTNG_SYMBOL_NAME_LEN_SCANF_IS_A_BROKEN_API + "[^'+']+%" S_HEX_LEN_SCANF_IS_A_BROKEN_API "s", name, s_hex); if (ret == 2) { strncpy(ev->attr.probe.symbol_name, name, LTTNG_SYMBOL_NAME_LEN); ev->attr.probe.symbol_name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0'; @@ -252,7 +258,8 @@ static int parse_probe_opts(struct lttng_event *ev, char *opt) /* Check for symbol */ if (isalpha(name[0])) { - ret = sscanf(opt, "%s", name); + ret = sscanf(opt, "%" LTTNG_SYMBOL_NAME_LEN_SCANF_IS_A_BROKEN_API "s", + name); if (ret == 1) { strncpy(ev->attr.probe.symbol_name, name, LTTNG_SYMBOL_NAME_LEN); ev->attr.probe.symbol_name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0'; @@ -265,7 +272,7 @@ static int parse_probe_opts(struct lttng_event *ev, char *opt) } /* Check for address */ - ret = sscanf(opt, "%s", s_hex); + ret = sscanf(opt, "%" S_HEX_LEN_SCANF_IS_A_BROKEN_API "s", s_hex); if (ret > 0) { if (*s_hex == '\0') { ERR("Invalid probe address %s", s_hex); diff --git a/src/bin/lttng/conf.c b/src/bin/lttng/conf.c index 5a0da9d33..55ed63526 100644 --- a/src/bin/lttng/conf.c +++ b/src/bin/lttng/conf.c @@ -186,6 +186,9 @@ char *config_read_session_name(char *path) int ret; FILE *fp; char var[NAME_MAX], *session_name; +#if (NAME_MAX == 255) +#define NAME_MAX_SCANF_IS_A_BROKEN_API "254" +#endif session_name = malloc(NAME_MAX); if (session_name == NULL) { @@ -202,7 +205,9 @@ char *config_read_session_name(char *path) } while (!feof(fp)) { - if ((ret = fscanf(fp, "%[^'=']=%s\n", var, session_name)) != 2) { + if ((ret = fscanf(fp, "%" NAME_MAX_SCANF_IS_A_BROKEN_API + "[^'=']=%" NAME_MAX_SCANF_IS_A_BROKEN_API "s\n", + var, session_name)) != 2) { if (ret == -1) { ERR("Missing session=NAME in config file."); goto error_close; diff --git a/src/lib/lttng-ctl/filter/filter-parser.y b/src/lib/lttng-ctl/filter/filter-parser.y index 29e2866a7..d746f78e8 100644 --- a/src/lib/lttng-ctl/filter/filter-parser.y +++ b/src/lib/lttng-ctl/filter/filter-parser.y @@ -34,6 +34,11 @@ #include +#define WIDTH_u64_SCANF_IS_A_BROKEN_API "20" +#define WIDTH_o64_SCANF_IS_A_BROKEN_API "22" +#define WIDTH_x64_SCANF_IS_A_BROKEN_API "17" +#define WIDTH_lg_SCANF_IS_A_BROKEN_API "4096" /* Hugely optimistic approximation */ + LTTNG_HIDDEN int yydebug; LTTNG_HIDDEN @@ -399,29 +404,39 @@ primary_expression { $$ = make_node(parser_ctx, NODE_EXPRESSION); $$->u.expression.type = AST_EXP_CONSTANT; - sscanf(yylval.gs->s, "%" PRIu64, - &$$->u.expression.u.constant); + if (sscanf(yylval.gs->s, "%" WIDTH_u64_SCANF_IS_A_BROKEN_API SCNu64, + &$$->u.expression.u.constant) != 1) { + parse_error(parser_ctx, "cannot scanf decimal constant"); + } } | OCTAL_CONSTANT { $$ = make_node(parser_ctx, NODE_EXPRESSION); $$->u.expression.type = AST_EXP_CONSTANT; - sscanf(yylval.gs->s, "0%" PRIo64, - &$$->u.expression.u.constant); + if (!strcmp(yylval.gs->s, "0")) { + $$->u.expression.u.constant = 0; + } else if (sscanf(yylval.gs->s, "0%" WIDTH_o64_SCANF_IS_A_BROKEN_API SCNo64, + &$$->u.expression.u.constant) != 1) { + parse_error(parser_ctx, "cannot scanf octal constant"); + } } | HEXADECIMAL_CONSTANT { $$ = make_node(parser_ctx, NODE_EXPRESSION); $$->u.expression.type = AST_EXP_CONSTANT; - sscanf(yylval.gs->s, "0x%" PRIx64, - &$$->u.expression.u.constant); + if (sscanf(yylval.gs->s, "0x%" WIDTH_x64_SCANF_IS_A_BROKEN_API SCNx64, + &$$->u.expression.u.constant) != 1) { + parse_error(parser_ctx, "cannot scanf hexadecimal constant"); + } } | FLOAT_CONSTANT { $$ = make_node(parser_ctx, NODE_EXPRESSION); $$->u.expression.type = AST_EXP_FLOAT_CONSTANT; - sscanf(yylval.gs->s, "%lg", - &$$->u.expression.u.float_constant); + if (sscanf(yylval.gs->s, "%" WIDTH_lg_SCANF_IS_A_BROKEN_API "lg", + &$$->u.expression.u.float_constant) != 1) { + parse_error(parser_ctx, "cannot scanf float constant"); + } } | STRING_LITERAL_START DQUOTE { -- 2.34.1