[OpenWrt-Devel] [PATCH v2 libubox 06/10] base64: fix possible null pointer dereference

[Petr Štetiar]

clang-10 analyzer reports following:

 base64.c:325:20: warning: Array access (from variable 'target') results in a null pointer dereference
                 target[tarindex] = 0;
                 ~~~~~~           ^

and prepared test case confirms it:

 Invalid write of size 1
    at 0x4E4463F: b64_decode (base64.c:325)
    by 0x40088C: test_invalid_inputs (tests/test-base64.c:26)
    by 0x40088C: main (tests/test-base64.c:32)
  Address 0x1 is not stack'd, malloc'd or (recently) free'd

 Process terminating with default action of signal 11 (SIGSEGV)
  Access not within mapped region at address 0x1
    at 0x4E4463F: b64_decode (base64.c:325)
    by 0x40088C: test_invalid_inputs (tests/test-base64.c:26)
    by 0x40088C: main (tests/test-base64.c:32)

Signed-off-by: Petr Štetiar <ynezz at true.cz>
---
 base64.c                  | 6 ++++++
 tests/CMakeLists.txt      | 2 ++
 tests/cram/CMakeLists.txt | 2 ++
 tests/cram/test_base64.t  | 8 ++++++++
 tests/test-b64_decode.c   | 7 +++++++
 tests/test-b64_encode.c   | 7 +++++++
 6 files changed, 32 insertions(+)
 create mode 100644 tests/test-b64_decode.c
 create mode 100644 tests/test-b64_encode.c

diff --git a/base64.c b/base64.c
index 4759ede01e7c..1bf21772fbed 100644
--- a/base64.c
+++ b/base64.c
@@ -65,6 +65,8 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+
+#include "assert.h"
 #include "utils.h"
 
 static const char Base64[] =
@@ -144,6 +146,8 @@ int b64_encode(const void *_src, size_t srclength,
 	u_char output[4];
 	size_t i;
 
+	assert(dest && targsize > 0);
+
 	while (2 < srclength) {
 		input[0] = *src++;
 		input[1] = *src++;
@@ -208,6 +212,8 @@ int b64_decode(const void *_src, void *dest, size_t targsize)
 	state = 0;
 	tarindex = 0;
 
+	assert(dest && targsize > 0);
+
 	while ((ch = (unsigned char)*src++) != '\0') {
 		if (isspace(ch))	/* Skip whitespace anywhere. */
 			continue;
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 6832b3f52c30..60d7b9839ad4 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -8,3 +8,5 @@ ENDMACRO(ADD_UNIT_TEST)
 
 ADD_UNIT_TEST(avl)
 ADD_UNIT_TEST(b64)
+ADD_UNIT_TEST(b64_encode)
+ADD_UNIT_TEST(b64_decode)
diff --git a/tests/cram/CMakeLists.txt b/tests/cram/CMakeLists.txt
index ca00d11dd686..2deb4ef3dd1f 100644
--- a/tests/cram/CMakeLists.txt
+++ b/tests/cram/CMakeLists.txt
@@ -25,3 +25,5 @@ ENDMACRO(ADD_CRAM_TEST_ENV)
 ADD_CRAM_TEST_ENV(JSHN jshn)
 ADD_CRAM_TEST_ENV(TEST_AVL test-avl)
 ADD_CRAM_TEST_ENV(TEST_B64 test-b64)
+ADD_CRAM_TEST_ENV(TEST_B64_ENCODE test-b64_encode)
+ADD_CRAM_TEST_ENV(TEST_B64_DECODE test-b64_decode)
diff --git a/tests/cram/test_base64.t b/tests/cram/test_base64.t
index 7e41442746d8..f58bbaa71acb 100644
--- a/tests/cram/test_base64.t
+++ b/tests/cram/test_base64.t
@@ -15,3 +15,11 @@ check that base64 is producing expected results:
   4 foob
   5 fooba
   6 foobar
+
+  $ $TEST_B64_DECODE 2>&1 | cut -d : -f4-
+   b64_decode: Assertion `dest && targsize > 0' failed.
+  Aborted (core dumped)
+
+  $ $TEST_B64_ENCODE 2>&1 | cut -d : -f4-
+   b64_encode: Assertion `dest && targsize > 0' failed.
+  Aborted (core dumped)
diff --git a/tests/test-b64_decode.c b/tests/test-b64_decode.c
new file mode 100644
index 000000000000..4798fa8da380
--- /dev/null
+++ b/tests/test-b64_decode.c
@@ -0,0 +1,7 @@
+#include "utils.h"
+
+int main()
+{
+	b64_decode("Zg==", NULL, 2);
+	return 0;
+}
diff --git a/tests/test-b64_encode.c b/tests/test-b64_encode.c
new file mode 100644
index 000000000000..5f011e52dec6
--- /dev/null
+++ b/tests/test-b64_encode.c
@@ -0,0 +1,7 @@
+#include "utils.h"
+
+int main()
+{
+	b64_encode("foo", 3, NULL, 2);
+	return 0;
+}

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel