1. openvpn
  2. Commit f5a2e2319bc7c3cce7f5cdccae44b9bae3e33375
Browse code

buffer: Fix buf_parse eating input

When parsing a "line" that is longer than the
available line buffer, then buf_parse was
eating up to 2 characters. It advanced past
them but they were not part of the output.

This can lead to unexpected results if buf_parse
is used in a while loop on unrestricted input,
like e.g. when reading configs (see in_src_get()
used for check_inline_file_via_buf()).

Change-Id: I3724660bf0f8336ee58c172acfb7c4f38e457393
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1246
Message-Id: <20251008103001.7696-1-gert@greenie.muc.de>
URL: https://sourceforge.net/p/openvpn/mailman/message/59243829/
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Frank Lichtenheld authored on 2025/10/08 19:29:55
Showing 3 changed files
src/openvpn/buffer.c
History View file @ f5a2e23
... ... 
@@ -832,19 +832,24 @@ buf_parse(struct buffer *buf, const int delim, char *line, const int size)
832 832
833 833 
     do
834 834 
     {
835 
-        c = buf_read_u8(buf);
835 
+        c = buf_peek_u8(buf);
836 836 
         if (c < 0)
837 837 
         {
838 838 
             eol = true;
839 
+            line[n] = 0;
840 
+            break;
839 841 
         }
840 
-        if (c <= 0 || c == delim)
842 
+        if (c == delim)
841 843 
         {
842 
-            c = 0;
844 
+            buf_advance(buf, 1);
845 
+            line[n] = 0;
846 
+            break;
843 847 
         }
844 
-        if (n >= size)
848 
+        if (n >= (size - 1))
845 849 
         {
846 850 
             break;
847 851 
         }
852 
+        buf_advance(buf, 1);
848 853 
         line[n++] = (char)c;
849 854 
     } while (c);
850 855
src/openvpn/buffer.h
History View file @ f5a2e23
... ... 
@@ -771,7 +771,7 @@ buf_read(struct buffer *src, void *dest, int size)
771 771 
 }
772 772
773 773 
 static inline int
774 
-buf_read_u8(struct buffer *buf)
774 
+buf_peek_u8(struct buffer *buf)
775 775 
 {
776 776 
     int ret;
777 777 
     if (BLEN(buf) < 1)
... ... 
@@ -779,7 +779,17 @@ buf_read_u8(struct buffer *buf)
779 779 
         return -1;
780 780 
     }
781 781 
     ret = *BPTR(buf);
782 
-    buf_advance(buf, 1);
782 
+    return ret;
783 
+}
784 
+
785 
+static inline int
786 
+buf_read_u8(struct buffer *buf)
787 
+{
788 
+    int ret = buf_peek_u8(buf);
789 
+    if (ret >= 0)
790 
+    {
791 
+        buf_advance(buf, 1);
792 
+    }
783 793 
     return ret;
784 794 
 }
785 795
tests/unit_tests/openvpn/test_buffer.c
History View file @ f5a2e23
... ... 
@@ -450,6 +450,56 @@ test_buffer_chomp(void **state)
450 450 
     gc_free(&gc);
451 451 
 }
452 452
453 
+/* for building long texts */
454 
+#define A_TIMES_256 "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAO"
455 
+
456 
+void
457 
+test_buffer_parse(void **state)
458 
+{
459 
+    struct gc_arena gc = gc_new();
460 
+    struct buffer buf = alloc_buf_gc(1024, &gc);
461 
+    char line[512];
462 
+    bool status;
463 
+    const char test1[] = A_TIMES_256 "EOL\n" A_TIMES_256 "EOF";
464 
+
465 
+    /* line buffer bigger than actual line */
466 
+    assert_int_equal(buf_write(&buf, test1, sizeof(test1)), true);
467 
+    status = buf_parse(&buf, '\n', line, sizeof(line));
468 
+    assert_int_equal(status, true);
469 
+    assert_string_equal(line, A_TIMES_256 "EOL");
470 
+    status = buf_parse(&buf, '\n', line, sizeof(line));
471 
+    assert_int_equal(status, true);
472 
+    assert_string_equal(line, A_TIMES_256 "EOF");
473 
+
474 
+    /* line buffer exactly same size as actual line + terminating \0 */
475 
+    buf_reset_len(&buf);
476 
+    assert_int_equal(buf_write(&buf, test1, sizeof(test1)), true);
477 
+    status = buf_parse(&buf, '\n', line, 260);
478 
+    assert_int_equal(status, true);
479 
+    assert_string_equal(line, A_TIMES_256 "EOL");
480 
+    status = buf_parse(&buf, '\n', line, 260);
481 
+    assert_int_equal(status, true);
482 
+    assert_string_equal(line, A_TIMES_256 "EOF");
483 
+
484 
+    /* line buffer smaller than actual line */
485 
+    buf_reset_len(&buf);
486 
+    assert_int_equal(buf_write(&buf, test1, sizeof(test1)), true);
487 
+    status = buf_parse(&buf, '\n', line, 257);
488 
+    assert_int_equal(status, true);
489 
+    assert_string_equal(line, A_TIMES_256);
490 
+    status = buf_parse(&buf, '\n', line, 257);
491 
+    assert_int_equal(status, true);
492 
+    assert_string_equal(line, "EOL");
493 
+    status = buf_parse(&buf, '\n', line, 257);
494 
+    assert_int_equal(status, true);
495 
+    assert_string_equal(line, A_TIMES_256);
496 
+    status = buf_parse(&buf, '\n', line, 257);
497 
+    assert_int_equal(status, true);
498 
+    assert_string_equal(line, "EOF");
499 
+
500 
+    gc_free(&gc);
501 
+}
502 
+
453 503 
 int
454 504 
 main(void)
455 505 
 {
... ... 
@@ -478,7 +528,8 @@ main(void)
478 478 
         cmocka_unit_test(test_character_class),
479 479 
         cmocka_unit_test(test_character_string_mod_buf),
480 480 
         cmocka_unit_test(test_snprintf),
481 
-        cmocka_unit_test(test_buffer_chomp)
481 
+        cmocka_unit_test(test_buffer_chomp),
482 
+        cmocka_unit_test(test_buffer_parse)
482 483 
     };
483 484
484 485 
     return cmocka_run_group_tests_name("buffer", tests, NULL, NULL);