From d45b1de81aa47cef4ca098a4b2725d855b154162 Mon Sep 17 00:00:00 2001 From: Philip Hazel Date: Mon, 18 Sep 2006 14:49:23 +0000 Subject: [PATCH] Check for overflow in numeric expansion conditions; forbid negative values for message_size_limit. --- doc/doc-txt/ChangeLog | 6 +++++- src/src/deliver.c | 6 +++--- src/src/exim.c | 6 +++--- src/src/expand.c | 38 ++++++++++++++---------------------- src/src/functions.h | 4 ++-- src/src/smtp_in.c | 6 +++--- test/scripts/0000-Basic/0002 | 7 +++++++ test/stdout/0002 | 11 +++++++++-- 8 files changed, 47 insertions(+), 37 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 976309142..cf5f158a7 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -1,4 +1,4 @@ -$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.391 2006/09/18 11:06:20 ph10 Exp $ +$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.392 2006/09/18 14:49:23 ph10 Exp $ Change log file for Exim from version 4.21 ------------------------------------------- @@ -38,6 +38,10 @@ PH/05 Applied Nico Efrurth's refactoring patch to tidy up spool_mbox.c. PH/06 Installed the latest Cygwin Makefile from the Cygwin maintainer. +PH/07 There was no check for overflow in expansions such as ${if >{1}{4096M}}. + +PH/08 An error is now given if message_size_limit is specified negative. + Exim version 4.63 ----------------- diff --git a/src/src/deliver.c b/src/src/deliver.c index fdff07414..33c8b85ae 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -1,4 +1,4 @@ -/* $Cambridge: exim/src/src/deliver.c,v 1.35 2006/07/04 09:07:20 ph10 Exp $ */ +/* $Cambridge: exim/src/src/deliver.c,v 1.36 2006/09/18 14:49:23 ph10 Exp $ */ /************************************************* * Exim - an Internet mail transport agent * @@ -1433,10 +1433,10 @@ int rc = OK; int size_limit; deliver_set_expansions(addr); -size_limit = expand_string_integer(tp->message_size_limit); +size_limit = expand_string_integer(tp->message_size_limit, TRUE); deliver_set_expansions(NULL); -if (size_limit < 0) +if (expand_string_message != NULL) { rc = DEFER; if (size_limit == -1) diff --git a/src/src/exim.c b/src/src/exim.c index 8c5c23eff..d209b767e 100644 --- a/src/src/exim.c +++ b/src/src/exim.c @@ -1,4 +1,4 @@ -/* $Cambridge: exim/src/src/exim.c,v 1.42 2006/07/27 10:13:52 ph10 Exp $ */ +/* $Cambridge: exim/src/src/exim.c,v 1.43 2006/09/18 14:49:23 ph10 Exp $ */ /************************************************* * Exim - an Internet mail transport agent * @@ -4444,8 +4444,8 @@ if (smtp_input) else { - thismessage_size_limit = expand_string_integer(message_size_limit); - if (thismessage_size_limit < 0) + thismessage_size_limit = expand_string_integer(message_size_limit, TRUE); + if (expand_string_message != NULL) { if (thismessage_size_limit == -1) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "failed to expand " diff --git a/src/src/expand.c b/src/src/expand.c index 5da6f99b8..1517c0625 100644 --- a/src/src/expand.c +++ b/src/src/expand.c @@ -1,4 +1,4 @@ -/* $Cambridge: exim/src/src/expand.c,v 1.59 2006/09/05 14:05:43 ph10 Exp $ */ +/* $Cambridge: exim/src/src/expand.c,v 1.60 2006/09/18 14:49:23 ph10 Exp $ */ /************************************************* * Exim - an Internet mail transport agent * @@ -1893,25 +1893,8 @@ switch(cond_type) if (!isalpha(name[0])) { - uschar *endptr; - num[i] = (int)Ustrtol((const uschar *)sub[i], &endptr, 10); - if (tolower(*endptr) == 'k') - { - num[i] *= 1024; - endptr++; - } - else if (tolower(*endptr) == 'm') - { - num[i] *= 1024*1024; - endptr++; - } - while (isspace(*endptr)) endptr++; - if (*endptr != 0) - { - expand_string_message = string_sprintf("\"%s\" is not a number", - sub[i]); - return NULL; - } + num[i] = expand_string_integer(sub[i], FALSE); + if (expand_string_message != NULL) return NULL; } } @@ -5260,22 +5243,26 @@ return yield; /* Expand a string, and convert the result into an integer. -Argument: the string to be expanded +Arguments: + string the string to be expanded + isplus TRUE if a non-negative number is expected Returns: the integer value, or -1 for an expansion error ) in both cases, message in -2 for an integer interpretation error ) expand_string_message - + expand_string_message is set NULL for an OK integer */ int -expand_string_integer(uschar *string) +expand_string_integer(uschar *string, BOOL isplus) { long int value; uschar *s = expand_string(string); uschar *msg = US"invalid integer \"%s\""; uschar *endptr; +/* If expansion failed, expand_string_message will be set. */ + if (s == NULL) return -1; /* On an overflow, strtol() returns LONG_MAX or LONG_MIN, and sets errno @@ -5283,12 +5270,17 @@ to ERANGE. When there isn't an overflow, errno is not changed, at least on some systems, so we set it zero ourselves. */ errno = 0; +expand_string_message = NULL; /* Indicates no error */ value = strtol(CS s, CSS &endptr, 0); if (endptr == s) { msg = US"integer expected but \"%s\" found"; } +else if (value < 0 && isplus) + { + msg = US"non-negative integer expected but \"%s\" found"; + } else { /* Ensure we can cast this down to an int */ diff --git a/src/src/functions.h b/src/src/functions.h index 32b3d2d24..eb1a7a5b4 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -1,4 +1,4 @@ -/* $Cambridge: exim/src/src/functions.h,v 1.25 2006/07/13 13:53:33 ph10 Exp $ */ +/* $Cambridge: exim/src/src/functions.h,v 1.26 2006/09/18 14:49:23 ph10 Exp $ */ /************************************************* * Exim - an Internet mail transport agent * @@ -97,7 +97,7 @@ extern void exim_wait_tick(struct timeval *, int); extern BOOL expand_check_condition(uschar *, uschar *, uschar *); extern uschar *expand_string(uschar *); extern uschar *expand_string_copy(uschar *); -extern int expand_string_integer(uschar *); +extern int expand_string_integer(uschar *, BOOL); extern int filter_interpret(uschar *, int, address_item **, uschar **); extern BOOL filter_personal(string_item *, BOOL); diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index 3b1f6a33e..6c27c15de 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -1,4 +1,4 @@ -/* $Cambridge: exim/src/src/smtp_in.c,v 1.41 2006/07/27 11:29:32 ph10 Exp $ */ +/* $Cambridge: exim/src/src/smtp_in.c,v 1.42 2006/09/18 14:49:23 ph10 Exp $ */ /************************************************* * Exim - an Internet mail transport agent * @@ -1211,8 +1211,8 @@ smtp_had_eof = smtp_had_error = 0; /* Set up the message size limit; this may be host-specific */ -thismessage_size_limit = expand_string_integer(message_size_limit); -if (thismessage_size_limit < 0) +thismessage_size_limit = expand_string_integer(message_size_limit, TRUE); +if (expand_string_message != NULL) { if (thismessage_size_limit == -1) log_write(0, LOG_MAIN|LOG_PANIC, "unable to expand message_size_limit: " diff --git a/test/scripts/0000-Basic/0002 b/test/scripts/0000-Basic/0002 index 251b8d0c5..19e1e856a 100644 --- a/test/scripts/0000-Basic/0002 +++ b/test/scripts/0000-Basic/0002 @@ -187,6 +187,11 @@ hash: ${if eq {1}{2}{${hash_3:invalid}}{NO}} md5: ${if eq {1}{2}{${md5:invalid}}{NO}} mask: ${if eq {1}{2}{${mask:invalid}}{NO}} +# Numeric overflow + +4096M ${if >{1}{4096M}{y}{n}} +4096000000 ${if >{1}{4096000000}{y}{n}} + # Conditions 2=2: ${if ={2}{2}{y}{n}} @@ -200,6 +205,7 @@ mask: ${if eq {1}{2}{${mask:invalid}}{NO}} 2>3: ${if >{2}{3}{y}{n}} 3>3: ${if >{3}{3}{y}{n}} 4>3: ${if >{4}{3}{y}{n}} +1>-1: ${if >{1}{-1}{y}{n}} 2>=3: ${if >={2}{3}{y}{n}} 3>=3: ${if >={3}{3}{y}{n}} 4>=3: ${if >={4}{3}{y}{n}} @@ -210,6 +216,7 @@ mask: ${if eq {1}{2}{${mask:invalid}}{NO}} 3<=3: ${if <={3}{3}{y}{n}} 4<=3: ${if <={4}{3}{y}{n}} 5<=3: ${if <={ 5 } { 3 } {y}{n}} +-3<=1: ${if <={-3}{1}{y}{n}} 5>3k: ${if >{5 } {3k }{y}{n}} 5>3m: ${if >{5 } {3m }{y}{n}} diff --git a/test/stdout/0002 b/test/stdout/0002 index e437faded..96d4047ac 100644 --- a/test/stdout/0002 +++ b/test/stdout/0002 @@ -168,6 +168,11 @@ > md5: NO > mask: NO > +> # Numeric overflow +> +> Failed: absolute value of integer "4096M" is too large (overflow) +> Failed: absolute value of integer "4096000000" is too large (overflow) +> > # Conditions > > 2=2: y @@ -181,6 +186,7 @@ > 2>3: n > 3>3: n > 4>3: y +> 1>-1: y > 2>=3: n > 3>=3: y > 4>=3: y @@ -191,11 +197,12 @@ > 3<=3: y > 4<=3: n > 5<=3: n +> -3<=1: y > > 5>3k: n > 5>3m: n -> Failed: "3z " is not a number -> Failed: "a" is not a number +> Failed: invalid integer "3z " +> Failed: integer expected but "a" found > > def:y y > def:n n -- 2.30.2