File 4352-Fix-ei_decode_iodata-illegal-write-and-improve-testc.patch of Package erlang
From 84ae4ec1819f254fdc4841d5d0436286104776dc Mon Sep 17 00:00:00 2001
From: Rickard Green <rickard@erlang.org>
Date: Wed, 22 Apr 2020 17:26:12 +0200
Subject: [PATCH] Fix ei_decode_iodata() illegal write and improve testcase
---
lib/erl_interface/src/decode/decode_iodata.c | 57 +++++++++++++------
lib/erl_interface/test/ei_decode_SUITE.erl | 1 +
.../test/ei_decode_SUITE_data/ei_decode_test.c | 64 ++++++++++++++++++++--
3 files changed, 100 insertions(+), 22 deletions(-)
diff --git a/lib/erl_interface/src/decode/decode_iodata.c b/lib/erl_interface/src/decode/decode_iodata.c
index 2a538d2ad7..88d23d8a1e 100644
--- a/lib/erl_interface/src/decode/decode_iodata.c
+++ b/lib/erl_interface/src/decode/decode_iodata.c
@@ -19,11 +19,15 @@
*
*/
+#include <string.h>
#include "eidef.h"
#include "eiext.h"
+#include "putget.h"
static int decode_list_ext_iodata(const char *buf, int* index,
int *szp, unsigned char **pp);
+static void decode_string(const char *buf, int* index,
+ int *szp, unsigned char **pp);
int ei_decode_iodata(const char *buf, int* index, int *szp, char *out_buf)
{
@@ -50,13 +54,11 @@ int ei_decode_iodata(const char *buf, int* index, int *szp, char *out_buf)
return 0;
}
- case ERL_STRING_EXT:
- if (ei_decode_string(buf, index, out_buf) < 0)
- return -1;
- if (szp)
- *szp += len;
+ case ERL_STRING_EXT: {
+ unsigned char *p = (unsigned char *) out_buf;
+ decode_string(buf, index, szp, p ? &p : NULL);
return 0;
-
+ }
case ERL_NIL_EXT:
return ei_decode_list_header(buf, index, NULL);
@@ -69,7 +71,7 @@ int ei_decode_iodata(const char *buf, int* index, int *szp, char *out_buf)
}
default:
- return -1; /* Not a list or binary... */
+ return -1; /* Not a list nor a binary... */
}
}
@@ -113,15 +115,9 @@ static int decode_list_ext_iodata(const char *buf, int* index,
*szp += len;
break;
}
- case ERL_STRING_EXT: {
- void *p = pp ? *pp : NULL;
- if (ei_decode_string(buf, index, p) < 0)
- return -1;
- if (pp)
- *pp += len;
- *szp += len;
+ case ERL_STRING_EXT:
+ decode_string(buf, index, szp, pp);
break;
- }
case ERL_LIST_EXT:
if (decode_list_ext_iodata(buf, index, szp, pp) < 0)
return -1;
@@ -131,7 +127,7 @@ static int decode_list_ext_iodata(const char *buf, int* index,
return -1;
break;
default:
- /* Not a list, binary, nor byte sized integer... */
+ /* Not a list, a binary, nor a byte sized integer... */
return -1;
}
}
@@ -139,3 +135,32 @@ static int decode_list_ext_iodata(const char *buf, int* index,
return 0;
}
+static void
+decode_string(const char *buf, int* index, int *szp, unsigned char **pp)
+{
+ /*
+ * ei_decode_string() null-terminates the string
+ * which we do not want, so we decode it ourselves
+ * here instead...
+ */
+ int len;
+ char *s = (char *) buf + *index;
+ char *s0 = s;
+
+ /* ASSERT(*s == ERL_STRING_EXT); */
+ s++;
+
+ len = get16be(s);
+
+ if (pp) {
+ memcpy(*pp, s, len);
+ *pp += len;
+ }
+
+ if (szp)
+ *szp += len;
+
+ s += len;
+ *index += s-s0;
+
+}
diff --git a/lib/erl_interface/test/ei_decode_SUITE.erl b/lib/erl_interface/test/ei_decode_SUITE.erl
index 37b51329ea..cde6a687eb 100644
--- a/lib/erl_interface/test/ei_decode_SUITE.erl
+++ b/lib/erl_interface/test/ei_decode_SUITE.erl
@@ -228,6 +228,7 @@ test_ei_decode_iodata(Config) when is_list(Config) ->
check_decode_iodata(P, [$a|$a], false),
check_decode_iodata(P, [[$a|$a],$a], false),
check_decode_iodata(P, "hej", true),
+ check_decode_iodata(P, ["hej", " ", "hopp"], true),
check_decode_iodata(P, <<"hopp san sa">>, true),
check_decode_iodata(P, [$a | <<"a">>], true),
check_decode_iodata(P, [[[["hej"]]], [$ , <<"hopp">>, $ , "san" | <<" sa">>]], true),
diff --git a/lib/erl_interface/test/ei_decode_SUITE_data/ei_decode_test.c b/lib/erl_interface/test/ei_decode_SUITE_data/ei_decode_test.c
index 961137f36c..bf51e3db87 100644
--- a/lib/erl_interface/test/ei_decode_SUITE_data/ei_decode_test.c
+++ b/lib/erl_interface/test/ei_decode_SUITE_data/ei_decode_test.c
@@ -756,12 +756,16 @@ TESTCASE(test_ei_decode_utf8_atom)
TESTCASE(test_ei_decode_iodata)
{
+ char *buf = NULL, *data = NULL;
ei_init();
while (1) {
- char *buf, *data;
+ int unexpected_write = 0;
+ int i;
int len, index, saved_index, err;
+ if (buf)
+ free_packet(buf);
buf = read_packet(&len);
if (len == 4
@@ -774,8 +778,10 @@ TESTCASE(test_ei_decode_iodata)
index = 0;
err = ei_decode_version(buf, &index, NULL);
- if (err != 0)
+ if (err != 0) {
+ free_packet(buf);
fail1("ei_decode_version returned %d", err);
+ }
saved_index = index;
err = ei_decode_iodata(buf, &index, &len, NULL);
if (err != 0) {
@@ -786,7 +792,22 @@ TESTCASE(test_ei_decode_iodata)
ei_x_free(&x);
continue;
}
- data = malloc(len);
+ if (data) {
+ data -= 100;
+ free(data);
+ }
+ data = malloc(len + 200);
+ if (!data) {
+ ei_x_buff x;
+ ei_x_new_with_version(&x);
+ ei_x_encode_atom(&x, "malloc_failed");
+ send_bin_term(&x);
+ ei_x_free(&x);
+ continue;
+ }
+ for (i = 0; i < len + 200; i++)
+ data[i] = 'Y';
+ data += 100;
err = ei_decode_iodata(buf, &saved_index, NULL, (unsigned char *) data);
if (err != 0) {
ei_x_buff x;
@@ -794,14 +815,45 @@ TESTCASE(test_ei_decode_iodata)
ei_x_encode_atom(&x, "decode_data_failed");
send_bin_term(&x);
ei_x_free(&x);
- free(data);
continue;
}
- send_buffer(data, len);
- free(data);
+ for (i = -100; i < 0; i++) {
+ if (data[i] != 'Y') {
+ ei_x_buff x;
+ ei_x_new_with_version(&x);
+ ei_x_encode_atom(&x, "unexpected_write_before_data");
+ send_bin_term(&x);
+ ei_x_free(&x);
+ unexpected_write = !0;
+ break;
+ }
+ }
+
+ if (!unexpected_write) {
+ for (i = len; i < len + 100; i++) {
+ if (data[i] != 'Y') {
+ ei_x_buff x;
+ ei_x_new_with_version(&x);
+ ei_x_encode_atom(&x, "unexpected_write_after_data");
+ send_bin_term(&x);
+ ei_x_free(&x);
+ unexpected_write = !0;
+ break;
+ }
+ }
+ }
+
+ if (!unexpected_write)
+ send_buffer(data, len);
}
+ if (buf)
+ free_packet(buf);
+ if (data) {
+ data -= 100;
+ free(data);
+ }
report(1);
}
--
2.16.4
