Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mjson_print_fixed_buf writes out of bounds on zero-length output buffer #48

Open
dauc opened this issue Aug 18, 2022 · 0 comments
Open

Comments

@dauc
Copy link

dauc commented Aug 18, 2022

When passing a zero length output buffer, mjson_print_fixed_buf (specifically, this line https://github.com/cesanta/mjson/blob/master/src/mjson.c#L479) writes out of bounds fb->len is -1.

I would expect to not write anything, but writing before the beginning of the output buffer is extra bad.

I happened to exercise this with something like mjson_snprintf(NULL, 0, "foo");.

See below patch for a unit test that exercises the issue.

diff --git a/test/unit_test.c b/test/unit_test.c
index 93bdd36..14579ed 100644
--- a/test/unit_test.c
+++ b/test/unit_test.c
@@ -412,6 +412,19 @@ static void test_print(void) {
     ASSERT(memcmp(tmp, str, 15) == 0);
     ASSERT(fb.len < fb.size);
   }
+
+    {
+    struct mjson_fixedbuf fb = {&tmp[1], 0, 0};
+    tmp[0] = (char)0xA5; // arbitrary non-ascii value
+    const char *s = "a\b\n\f\r\t\"";
+    // Unsure about what I would expect the return value to be
+    ASSERT(mjson_print_str(&mjson_print_fixed_buf, &fb, s, 7) == -1);
+    // wrote a null to buf[-1]
+    ASSERT(tmp[0] == '\0');
+    // This is what we should actually expect if it didn't write out of bounds.  This fails
+    ASSERT(tmp[0] == (char)0xA5);
+    ASSERT(fb.len < fb.size);
+  }
 }
 
 static int f1(mjson_print_fn_t fn, void *fndata, va_list *ap) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant