Browse code

Ensure option array p[] is always NULL-terminated

Add one element (a terminating NULL pointer) to the array into
which parse_line() stores the arguments. This prevents that options
that traverse this array until a terminator is seen (for instance
options that call no_more_than_n_args) will peek beyond buffer bounds.
In the worst case this might lead to a crash (stack overflow, not
likely in practice).

Signed-off-by: Guido Vranken <guidovranken@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <CAO5O-EKCLjPpdKUH6cCoqoZDAfekSafpc7Ga55H2_5Hs4rBopg@mail.gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14757.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Guido Vranken authored on 2017/06/08 08:02:38
Showing 1 changed files
... ...
@@ -4531,7 +4531,7 @@ read_config_file(struct options *options,
4531 4531
     FILE *fp;
4532 4532
     int line_num;
4533 4533
     char line[OPTION_LINE_SIZE+1];
4534
-    char *p[MAX_PARMS];
4534
+    char *p[MAX_PARMS+1];
4535 4535
 
4536 4536
     ++level;
4537 4537
     if (level <= max_recursive_levels)
... ...
@@ -4563,7 +4563,7 @@ read_config_file(struct options *options,
4563 4563
                 {
4564 4564
                     offset = 3;
4565 4565
                 }
4566
-                if (parse_line(line + offset, p, SIZE(p), file, line_num, msglevel, &options->gc))
4566
+                if (parse_line(line + offset, p, SIZE(p)-1, file, line_num, msglevel, &options->gc))
4567 4567
                 {
4568 4568
                     bypass_doubledash(&p[0]);
4569 4569
                     check_inline_file_via_fp(fp, p, &options->gc);
... ...
@@ -4605,10 +4605,10 @@ read_config_string(const char *prefix,
4605 4605
 
4606 4606
     while (buf_parse(&multiline, '\n', line, sizeof(line)))
4607 4607
     {
4608
-        char *p[MAX_PARMS];
4608
+        char *p[MAX_PARMS+1];
4609 4609
         CLEAR(p);
4610 4610
         ++line_num;
4611
-        if (parse_line(line, p, SIZE(p), prefix, line_num, msglevel, &options->gc))
4611
+        if (parse_line(line, p, SIZE(p)-1, prefix, line_num, msglevel, &options->gc))
4612 4612
         {
4613 4613
             bypass_doubledash(&p[0]);
4614 4614
             check_inline_file_via_buf(&multiline, p, &options->gc);
... ...
@@ -4739,14 +4739,14 @@ apply_push_options(struct options *options,
4739 4739
 
4740 4740
     while (buf_parse(buf, ',', line, sizeof(line)))
4741 4741
     {
4742
-        char *p[MAX_PARMS];
4742
+        char *p[MAX_PARMS+1];
4743 4743
         CLEAR(p);
4744 4744
         ++line_num;
4745 4745
         if (!apply_pull_filter(options, line))
4746 4746
         {
4747 4747
             return false; /* Cause push/pull error and stop push processing */
4748 4748
         }
4749
-        if (parse_line(line, p, SIZE(p), file, line_num, msglevel, &options->gc))
4749
+        if (parse_line(line, p, SIZE(p)-1, file, line_num, msglevel, &options->gc))
4750 4750
         {
4751 4751
             add_option(options, p, file, line_num, 0, msglevel, permission_mask, option_types_found, es);
4752 4752
         }