From 35de6c8c24b53adab2e69456373aecbecbe54a95 Mon Sep 17 00:00:00 2001 From: Shawn Routhier Date: Wed, 6 Jun 2012 22:50:31 +0000 Subject: Fix some issues in the code for parsing and printing options. [ISC-Bugs #22625] - properly print options that have several fields followed by an array of something for example "fIa" [ISC-Bugs #27289] - properly parse options in declarations that have several fields followed by an array of something for example "fIa" [ISC-Bugs #27296] - properly determine if we parsed a 16 or 32 bit value in evaluate_numeric_expression (extract-int). [ISC-Bugs #27314] - properly parse a zero length option from a lease file. Thanks to Marius Tomaschewski from SUSE for the report and prototype patch for this ticket as well as ticket 27289. --- RELNOTES | 11 ++++++++++ client/dhclient.c | 19 ++++++++++++++---- common/options.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-------- common/parse.c | 23 +++++++++++++++++---- common/tables.c | 7 ++++--- common/tree.c | 37 ++++++++++++++++++++++------------ 6 files changed, 125 insertions(+), 32 deletions(-) diff --git a/RELNOTES b/RELNOTES index cb8271a4..9bb00221 100644 --- a/RELNOTES +++ b/RELNOTES @@ -57,6 +57,17 @@ work on other platforms. Please report any problems and suggested fixes to for reporting this issue. [ISC-Bugs #29062] +- Fix some issues in the code for parsing and printing options. + [ISC-Bugs #22625] - properly print options that have several fields + followed by an array of something for example "fIa" + [ISC-Bugs #27289] - properly parse options in declarations that have + several fields followed by an array of something for example "fIa" + [ISC-Bugs #27296] - properly determine if we parsed a 16 or 32 bit + value in evaluate_numeric_expression (extract-int). + [ISC-Bugs #27314] - properly parse a zero length option from + a lease file. Thanks to Marius Tomaschewski from SUSE for the report + and prototype patch for this ticket as well as ticket 27289. + Changes since 4.2.3 ! Add a check for a null pointer before calling the regexec function. diff --git a/client/dhclient.c b/client/dhclient.c index a68e8859..f38a155d 100644 --- a/client/dhclient.c +++ b/client/dhclient.c @@ -2742,10 +2742,21 @@ void write_lease_option (struct option_cache *oc, } if (evaluate_option_cache (&ds, packet, lease, client_state, in_options, cfg_options, scope, oc, MDL)) { - fprintf(leaseFile, "%soption %s%s%s %s;\n", preamble, - name, dot, oc->option->name, - pretty_print_option(oc->option, ds.data, ds.len, - 1, 1)); + /* The option name */ + fprintf(leaseFile, "%soption %s%s%s", preamble, + name, dot, oc->option->name); + + /* The option value if there is one */ + if ((oc->option->format == NULL) || + (oc->option->format[0] != 'Z')) { + fprintf(leaseFile, " %s", + pretty_print_option(oc->option, ds.data, + ds.len, 1, 1)); + } + + /* The closing semi-colon and newline */ + fprintf(leaseFile, ";\n"); + data_string_forget (&ds, MDL); } } diff --git a/common/options.c b/common/options.c index e21992d2..ce2c08ae 100644 --- a/common/options.c +++ b/common/options.c @@ -1683,6 +1683,8 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes) const unsigned char *dp = data; char comma; unsigned long tval; + isc_boolean_t a_array = ISC_FALSE; + int len_used; if (emit_commas) comma = ','; @@ -1707,6 +1709,8 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes) fmtbuf [l] = option -> format [i]; switch (option -> format [i]) { case 'a': + a_array = ISC_TRUE; + /* Fall through */ case 'A': --numelem; fmtbuf [l] = 0; @@ -1735,6 +1739,8 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes) hunksize++; comma = ':'; numhunk = 0; + a_array = ISC_TRUE; + hunkinc = 1; } fmtbuf [l + 1] = 0; break; @@ -1828,13 +1834,34 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes) len - hunksize); /* If this is an array, compute its size. */ - if (!numhunk) - numhunk = len / hunksize; - /* See if we got an exact number of hunks. */ - if (numhunk > 0 && numhunk * hunksize < len) - log_error ("%s: %d extra bytes at end of array\n", - option -> name, - len - numhunk * hunksize); + if (numhunk == 0) { + if (a_array == ISC_TRUE) { + /* + * It is an 'a' type array - we repeat the + * last format type. A binary string for 'X' + * is also like this. hunkinc is the size + * of the last format type and we add 1 to + * cover the entire first record. + */ + numhunk = ((len - hunksize) / hunkinc) + 1; + len_used = hunksize + ((numhunk - 1) * hunkinc); + } else { + /* + * It is an 'A' type array - we repeat the + * entire record + */ + numhunk = len / hunksize; + len_used = numhunk * hunksize; + } + + /* See if we got an exact number of hunks. */ + if (len_used < len) { + log_error ("%s: %d extra bytes at end of array\n", + option -> name, + len - len_used); + } + } + /* A one-hunk array prints the same as a single hunk. */ if (numhunk < 0) @@ -1842,7 +1869,24 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes) /* Cycle through the array (or hunk) printing the data. */ for (i = 0; i < numhunk; i++) { - for (j = 0; j < numelem; j++) { + if ((a_array == ISC_TRUE) && (i != 0) && (numelem > 0)) { + /* + * For 'a' type of arrays we repeat + * only the last format character + * We should never hit the case of numelem == 0 + * but let's include the check to be safe. + */ + j = numelem - 1; + } else { + /* + * for other types of arrays or the first + * time through for 'a' types, we go through + * the entire set of format characters. + */ + j = 0; + } + + for (; j < numelem; j++) { switch (fmtbuf [j]) { case 't': /* endbuf-1 leaves room for NULL. */ diff --git a/common/parse.c b/common/parse.c index 434085a2..f72c0d65 100644 --- a/common/parse.c +++ b/common/parse.c @@ -5518,11 +5518,26 @@ int parse_option_decl (oc, cfile) if (status != ISC_R_SUCCESS || option == NULL) return 0; + fmt = option->format; + /* Parse the option data... */ do { - for (fmt = option -> format; *fmt; fmt++) { - if (*fmt == 'A') + for (; *fmt; fmt++) { + if (*fmt == 'A') { + /* 'A' is an array of records, start at + * the beginning + */ + fmt = option->format; + break; + } + + if (*fmt == 'a') { + /* 'a' is an array of the last field, + * back up one format character + */ + fmt--; break; + } if (*fmt == 'o' && fmt != option -> format) continue; switch (*fmt) { @@ -5718,7 +5733,7 @@ int parse_option_decl (oc, cfile) goto alloc; case 'Z': /* Zero-length option */ - token = next_token(&val, (unsigned *)0, cfile); + token = peek_token(&val, (unsigned *)0, cfile); if (token != SEMI) { parse_warn(cfile, "semicolon expected."); @@ -5735,7 +5750,7 @@ int parse_option_decl (oc, cfile) } } token = next_token (&val, (unsigned *)0, cfile); - } while (*fmt == 'A' && token == COMMA); + } while (*fmt && token == COMMA); if (token != SEMI) { parse_warn (cfile, "semicolon expected."); diff --git a/common/tables.c b/common/tables.c index c820d83b..d224dc1b 100644 --- a/common/tables.c +++ b/common/tables.c @@ -64,9 +64,10 @@ HASH_FUNCTIONS (option_code, const unsigned *, struct option, some event. The special all-ones value means 'infinite'. May either be printed as a decimal, eg, "3600", or as this name, eg, "infinite". f - flag (true or false) - A - array of whatever precedes (e.g., IA means array of IP addresses) - a - array of the preceding character (e.g., IIa means two or more IP - addresses) + A - array of all that precedes (e.g., fIA means array of records of + a flag and an IP address) + a - array of the preceding character (e.g., fIa means a single flag + followed by an array of IP addresses) U - name of an option space (universe) F - implicit flag - the presence of the option indicates that the flag is true. diff --git a/common/tree.c b/common/tree.c index 3c978b09..c82ac271 100644 --- a/common/tree.c +++ b/common/tree.c @@ -3,7 +3,7 @@ Routines for manipulating parse trees... */ /* - * Copyright (c) 2011 by Internet Systems Consortium, Inc. ("ISC") + * Copyright (c) 2011-2012 by Internet Systems Consortium, Inc. ("ISC") * Copyright (c) 2004-2007,2009 by Internet Systems Consortium, Inc. ("ISC") * Copyright (c) 1995-2003 by Internet Software Consortium * @@ -2409,6 +2409,7 @@ int evaluate_numeric_expression (result, packet, lease, client_state, struct binding *binding; struct binding_value *bv; unsigned long ileft, iright; + int rc = 0; switch (expr -> op) { case expr_check: @@ -2477,32 +2478,42 @@ int evaluate_numeric_expression (result, packet, lease, client_state, status = (evaluate_data_expression (&data, packet, lease, client_state, in_options, cfg_options, scope, expr -> data.extract_int, MDL)); - if (status && data.len >= 2) + if (status && data.len >= 2) { *result = getUShort (data.data); + rc = 1; + } #if defined (DEBUG_EXPRESSIONS) - log_debug ("num: extract_int16 (%s) = %ld", - ((status && data.len >= 2) ? - print_hex_1 (data.len, data.data, 60) : "NULL"), - *result); + if (rc == 1) { + log_debug ("num: extract_int16 (%s) = %ld", + print_hex_1(data.len, data.data, 60) + *result); + } else { + log_debug ("num: extract_int16 (NULL) = NULL"); + } #endif if (status) data_string_forget (&data, MDL); - return (status && data.len >= 2); + return (rc); case expr_extract_int32: memset (&data, 0, sizeof data); status = (evaluate_data_expression (&data, packet, lease, client_state, in_options, cfg_options, scope, expr -> data.extract_int, MDL)); - if (status && data.len >= 4) + if (status && data.len >= 4) { *result = getULong (data.data); + rc = 1; + } #if defined (DEBUG_EXPRESSIONS) - log_debug ("num: extract_int32 (%s) = %ld", - ((status && data.len >= 4) ? - print_hex_1 (data.len, data.data, 60) : "NULL"), - *result); + if (rc == 1) { + log_debug ("num: extract_int32 (%s) = %ld", + print_hex_1 (data.len, data.data, 60), + *result); + } else { + log_debug ("num: extract_int32 (NULL) = NULL"); + } #endif if (status) data_string_forget (&data, MDL); - return (status && data.len >= 4); + return (rc); case expr_const_int: *result = expr -> data.const_int; -- cgit v1.2.1