From ee6b68ce4c67870f9323d2a380eb949f447c56ee Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Mon, 1 May 2017 14:38:49 -0400 Subject: [PATCH 1/1] Fix a stack smash It occurs when memory compares are made that are larger than the on stack temporary buffers (either malloced or supplied). Rework the test test so it doesn't use a macro with a branch. Reviewed-by: Richard Levitte Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3155) --- test/test_test.c | 298 +++++++++++++++++++++++------------------- test/testutil/tests.c | 71 ++++++---- 2 files changed, 207 insertions(+), 162 deletions(-) diff --git a/test/test_test.c b/test/test_test.c index 879a051a59..9f365d4936 100644 --- a/test/test_test.c +++ b/test/test_test.c @@ -21,28 +21,34 @@ #include "e_os.h" #include "testutil.h" -#define C(l, b, t) \ - if ((t) != b) { \ - fprintf(stderr, "FATAL : %s != %d\n", #t, b); \ - goto l; \ +#define TEST(expected, test) test_case((expected), #test, (test)) + +static int test_case(int expected, const char *test, int result) +{ + if (result != expected) { + fprintf(stderr, "FATAL: %s != %d\n", test, expected); + return 0; } + return 1; +} static int test_int(void) { - C(err, 1, TEST_int_eq(1, 1)); - C(err, 0, TEST_int_eq(1, -1)); - C(err, 1, TEST_int_ne(1, 2)); - C(err, 0, TEST_int_ne(3, 3)); - C(err, 1, TEST_int_lt(4, 9)); - C(err, 0, TEST_int_lt(9, 4)); - C(err, 1, TEST_int_le(4, 9)); - C(err, 1, TEST_int_le(5, 5)); - C(err, 0, TEST_int_le(9, 4)); - C(err, 1, TEST_int_gt(8, 5)); - C(err, 0, TEST_int_gt(5, 8)); - C(err, 1, TEST_int_ge(8, 5)); - C(err, 1, TEST_int_ge(6, 6)); - C(err, 0, TEST_int_ge(5, 8)); + if (!TEST(1, TEST_int_eq(1, 1)) + | !TEST(0, TEST_int_eq(1, -1)) + | !TEST(1, TEST_int_ne(1, 2)) + | !TEST(0, TEST_int_ne(3, 3)) + | !TEST(1, TEST_int_lt(4, 9)) + | !TEST(0, TEST_int_lt(9, 4)) + | !TEST(1, TEST_int_le(4, 9)) + | !TEST(1, TEST_int_le(5, 5)) + | !TEST(0, TEST_int_le(9, 4)) + | !TEST(1, TEST_int_gt(8, 5)) + | !TEST(0, TEST_int_gt(5, 8)) + | !TEST(1, TEST_int_ge(8, 5)) + | !TEST(1, TEST_int_ge(6, 6)) + | !TEST(0, TEST_int_ge(5, 8))) + goto err; return 1; err: @@ -51,20 +57,21 @@ err: static int test_uint(void) { - C(err, 1, TEST_uint_eq(3u, 3u)); - C(err, 0, TEST_uint_eq(3u, 5u)); - C(err, 1, TEST_uint_ne(4u, 2u)); - C(err, 0, TEST_uint_ne(6u, 6u)); - C(err, 1, TEST_uint_lt(5u, 9u)); - C(err, 0, TEST_uint_lt(9u, 5u)); - C(err, 1, TEST_uint_le(5u, 9u)); - C(err, 1, TEST_uint_le(7u, 7u)); - C(err, 0, TEST_uint_le(9u, 5u)); - C(err, 1, TEST_uint_gt(11u, 1u)); - C(err, 0, TEST_uint_gt(1u, 11u)); - C(err, 1, TEST_uint_ge(11u, 1u)); - C(err, 1, TEST_uint_ge(6u, 6u)); - C(err, 0, TEST_uint_ge(1u, 11u)); + if (!TEST(1, TEST_uint_eq(3u, 3u)) + | !TEST(0, TEST_uint_eq(3u, 5u)) + | !TEST(1, TEST_uint_ne(4u, 2u)) + | !TEST(0, TEST_uint_ne(6u, 6u)) + | !TEST(1, TEST_uint_lt(5u, 9u)) + | !TEST(0, TEST_uint_lt(9u, 5u)) + | !TEST(1, TEST_uint_le(5u, 9u)) + | !TEST(1, TEST_uint_le(7u, 7u)) + | !TEST(0, TEST_uint_le(9u, 5u)) + | !TEST(1, TEST_uint_gt(11u, 1u)) + | !TEST(0, TEST_uint_gt(1u, 11u)) + | !TEST(1, TEST_uint_ge(11u, 1u)) + | !TEST(1, TEST_uint_ge(6u, 6u)) + | !TEST(0, TEST_uint_ge(1u, 11u))) + goto err; return 1; err: @@ -73,20 +80,21 @@ err: static int test_char(void) { - C(err, 1, TEST_char_eq('a', 'a')); - C(err, 0, TEST_char_eq('a', 'A')); - C(err, 1, TEST_char_ne('a', 'c')); - C(err, 0, TEST_char_ne('e', 'e')); - C(err, 1, TEST_char_lt('i', 'x')); - C(err, 0, TEST_char_lt('x', 'i')); - C(err, 1, TEST_char_le('i', 'x')); - C(err, 1, TEST_char_le('n', 'n')); - C(err, 0, TEST_char_le('x', 'i')); - C(err, 1, TEST_char_gt('w', 'n')); - C(err, 0, TEST_char_gt('n', 'w')); - C(err, 1, TEST_char_ge('w', 'n')); - C(err, 1, TEST_char_ge('p', 'p')); - C(err, 0, TEST_char_ge('n', 'w')); + if (!TEST(1, TEST_char_eq('a', 'a')) + | !TEST(0, TEST_char_eq('a', 'A')) + | !TEST(1, TEST_char_ne('a', 'c')) + | !TEST(0, TEST_char_ne('e', 'e')) + | !TEST(1, TEST_char_lt('i', 'x')) + | !TEST(0, TEST_char_lt('x', 'i')) + | !TEST(1, TEST_char_le('i', 'x')) + | !TEST(1, TEST_char_le('n', 'n')) + | !TEST(0, TEST_char_le('x', 'i')) + | !TEST(1, TEST_char_gt('w', 'n')) + | !TEST(0, TEST_char_gt('n', 'w')) + | !TEST(1, TEST_char_ge('w', 'n')) + | !TEST(1, TEST_char_ge('p', 'p')) + | !TEST(0, TEST_char_ge('n', 'w'))) + goto err; return 1; err: @@ -95,20 +103,21 @@ err: static int test_uchar(void) { - C(err, 1, TEST_uchar_eq(49, 49)); - C(err, 0, TEST_uchar_eq(49, 60)); - C(err, 1, TEST_uchar_ne(50, 2)); - C(err, 0, TEST_uchar_ne(66, 66)); - C(err, 1, TEST_uchar_lt(60, 80)); - C(err, 0, TEST_uchar_lt(80, 60)); - C(err, 1, TEST_uchar_le(60, 80)); - C(err, 1, TEST_uchar_le(78, 78)); - C(err, 0, TEST_uchar_le(80, 60)); - C(err, 1, TEST_uchar_gt(88, 37)); - C(err, 0, TEST_uchar_gt(37, 88)); - C(err, 1, TEST_uchar_ge(88, 37)); - C(err, 1, TEST_uchar_ge(66, 66)); - C(err, 0, TEST_uchar_ge(37, 88)); + if (!TEST(1, TEST_uchar_eq(49, 49)) + | !TEST(0, TEST_uchar_eq(49, 60)) + | !TEST(1, TEST_uchar_ne(50, 2)) + | !TEST(0, TEST_uchar_ne(66, 66)) + | !TEST(1, TEST_uchar_lt(60, 80)) + | !TEST(0, TEST_uchar_lt(80, 60)) + | !TEST(1, TEST_uchar_le(60, 80)) + | !TEST(1, TEST_uchar_le(78, 78)) + | !TEST(0, TEST_uchar_le(80, 60)) + | !TEST(1, TEST_uchar_gt(88, 37)) + | !TEST(0, TEST_uchar_gt(37, 88)) + | !TEST(1, TEST_uchar_ge(88, 37)) + | !TEST(1, TEST_uchar_ge(66, 66)) + | !TEST(0, TEST_uchar_ge(37, 88))) + goto err; return 1; err: @@ -117,20 +126,21 @@ err: static int test_long(void) { - C(err, 1, TEST_long_eq(123l, 123l)); - C(err, 0, TEST_long_eq(123l, -123l)); - C(err, 1, TEST_long_ne(123l, 500l)); - C(err, 0, TEST_long_ne(1000l, 1000l)); - C(err, 1, TEST_long_lt(-8923l, 102934563l)); - C(err, 0, TEST_long_lt(102934563l, -8923l)); - C(err, 1, TEST_long_le(-8923l, 102934563l)); - C(err, 1, TEST_long_le(12345l, 12345l)); - C(err, 0, TEST_long_le(102934563l, -8923l)); - C(err, 1, TEST_long_gt(84325677l, 12345l)); - C(err, 0, TEST_long_gt(12345l, 84325677l)); - C(err, 1, TEST_long_ge(84325677l, 12345l)); - C(err, 1, TEST_long_ge(465869l, 465869l)); - C(err, 0, TEST_long_ge(12345l, 84325677l)); + if (!TEST(1, TEST_long_eq(123l, 123l)) + | !TEST(0, TEST_long_eq(123l, -123l)) + | !TEST(1, TEST_long_ne(123l, 500l)) + | !TEST(0, TEST_long_ne(1000l, 1000l)) + | !TEST(1, TEST_long_lt(-8923l, 102934563l)) + | !TEST(0, TEST_long_lt(102934563l, -8923l)) + | !TEST(1, TEST_long_le(-8923l, 102934563l)) + | !TEST(1, TEST_long_le(12345l, 12345l)) + | !TEST(0, TEST_long_le(102934563l, -8923l)) + | !TEST(1, TEST_long_gt(84325677l, 12345l)) + | !TEST(0, TEST_long_gt(12345l, 84325677l)) + | !TEST(1, TEST_long_ge(84325677l, 12345l)) + | !TEST(1, TEST_long_ge(465869l, 465869l)) + | !TEST(0, TEST_long_ge(12345l, 84325677l))) + goto err; return 1; err: @@ -139,20 +149,21 @@ err: static int test_ulong(void) { - C(err, 1, TEST_ulong_eq(919ul, 919ul)); - C(err, 0, TEST_ulong_eq(919ul, 10234ul)); - C(err, 1, TEST_ulong_ne(8190ul, 66ul)); - C(err, 0, TEST_ulong_ne(10555ul, 10555ul)); - C(err, 1, TEST_ulong_lt(10234ul, 1000000ul)); - C(err, 0, TEST_ulong_lt(1000000ul, 10234ul)); - C(err, 1, TEST_ulong_le(10234ul, 1000000ul)); - C(err, 1, TEST_ulong_le(100000ul, 100000ul)); - C(err, 0, TEST_ulong_le(1000000ul, 10234ul)); - C(err, 1, TEST_ulong_gt(100000000ul, 22ul)); - C(err, 0, TEST_ulong_gt(22ul, 100000000ul)); - C(err, 1, TEST_ulong_ge(100000000ul, 22ul)); - C(err, 1, TEST_ulong_ge(10555ul, 10555ul)); - C(err, 0, TEST_ulong_ge(22ul, 100000000ul)); + if (!TEST(1, TEST_ulong_eq(919ul, 919ul)) + | !TEST(0, TEST_ulong_eq(919ul, 10234ul)) + | !TEST(1, TEST_ulong_ne(8190ul, 66ul)) + | !TEST(0, TEST_ulong_ne(10555ul, 10555ul)) + | !TEST(1, TEST_ulong_lt(10234ul, 1000000ul)) + | !TEST(0, TEST_ulong_lt(1000000ul, 10234ul)) + | !TEST(1, TEST_ulong_le(10234ul, 1000000ul)) + | !TEST(1, TEST_ulong_le(100000ul, 100000ul)) + | !TEST(0, TEST_ulong_le(1000000ul, 10234ul)) + | !TEST(1, TEST_ulong_gt(100000000ul, 22ul)) + | !TEST(0, TEST_ulong_gt(22ul, 100000000ul)) + | !TEST(1, TEST_ulong_ge(100000000ul, 22ul)) + | !TEST(1, TEST_ulong_ge(10555ul, 10555ul)) + | !TEST(0, TEST_ulong_ge(22ul, 100000000ul))) + goto err; return 1; err: @@ -161,20 +172,21 @@ err: static int test_size_t(void) { - C(err, 1, TEST_int_eq((size_t)10, (size_t)10)); - C(err, 0, TEST_int_eq((size_t)10, (size_t)12)); - C(err, 1, TEST_int_ne((size_t)10, (size_t)12)); - C(err, 0, TEST_int_ne((size_t)24, (size_t)24)); - C(err, 1, TEST_int_lt((size_t)30, (size_t)88)); - C(err, 0, TEST_int_lt((size_t)88, (size_t)30)); - C(err, 1, TEST_int_le((size_t)30, (size_t)88)); - C(err, 1, TEST_int_le((size_t)33, (size_t)33)); - C(err, 0, TEST_int_le((size_t)88, (size_t)30)); - C(err, 1, TEST_int_gt((size_t)52, (size_t)33)); - C(err, 0, TEST_int_gt((size_t)33, (size_t)52)); - C(err, 1, TEST_int_ge((size_t)52, (size_t)33)); - C(err, 1, TEST_int_ge((size_t)38, (size_t)38)); - C(err, 0, TEST_int_ge((size_t)33, (size_t)52)); + if (!TEST(1, TEST_int_eq((size_t)10, (size_t)10)) + | !TEST(0, TEST_int_eq((size_t)10, (size_t)12)) + | !TEST(1, TEST_int_ne((size_t)10, (size_t)12)) + | !TEST(0, TEST_int_ne((size_t)24, (size_t)24)) + | !TEST(1, TEST_int_lt((size_t)30, (size_t)88)) + | !TEST(0, TEST_int_lt((size_t)88, (size_t)30)) + | !TEST(1, TEST_int_le((size_t)30, (size_t)88)) + | !TEST(1, TEST_int_le((size_t)33, (size_t)33)) + | !TEST(0, TEST_int_le((size_t)88, (size_t)30)) + | !TEST(1, TEST_int_gt((size_t)52, (size_t)33)) + | !TEST(0, TEST_int_gt((size_t)33, (size_t)52)) + | !TEST(1, TEST_int_ge((size_t)52, (size_t)33)) + | !TEST(1, TEST_int_ge((size_t)38, (size_t)38)) + | !TEST(0, TEST_int_ge((size_t)33, (size_t)52))) + goto err; return 1; err: @@ -186,20 +198,21 @@ static int test_pointer(void) int x = 0; char y = 1; - C(err, 1, TEST_ptr(&y)); - C(err, 0, TEST_ptr(NULL)); - C(err, 0, TEST_ptr_null(&y)); - C(err, 1, TEST_ptr_null(NULL)); - C(err, 1, TEST_ptr_eq(NULL, NULL)); - C(err, 0, TEST_ptr_eq(NULL, &y)); - C(err, 0, TEST_ptr_eq(&y, NULL)); - C(err, 0, TEST_ptr_eq(&y, &x)); - C(err, 1, TEST_ptr_eq(&x, &x)); - C(err, 0, TEST_ptr_ne(NULL, NULL)); - C(err, 1, TEST_ptr_ne(NULL, &y)); - C(err, 1, TEST_ptr_ne(&y, NULL)); - C(err, 1, TEST_ptr_ne(&y, &x)); - C(err, 0, TEST_ptr_ne(&x, &x)); + if (!TEST(1, TEST_ptr(&y)) + | !TEST(0, TEST_ptr(NULL)) + | !TEST(0, TEST_ptr_null(&y)) + | !TEST(1, TEST_ptr_null(NULL)) + | !TEST(1, TEST_ptr_eq(NULL, NULL)) + | !TEST(0, TEST_ptr_eq(NULL, &y)) + | !TEST(0, TEST_ptr_eq(&y, NULL)) + | !TEST(0, TEST_ptr_eq(&y, &x)) + | !TEST(1, TEST_ptr_eq(&x, &x)) + | !TEST(0, TEST_ptr_ne(NULL, NULL)) + | !TEST(1, TEST_ptr_ne(NULL, &y)) + | !TEST(1, TEST_ptr_ne(&y, NULL)) + | !TEST(1, TEST_ptr_ne(&y, &x)) + | !TEST(0, TEST_ptr_ne(&x, &x))) + goto err; return 1; err: @@ -208,10 +221,11 @@ err: static int test_bool(void) { - C(err, 0, TEST_true(0)); - C(err, 1, TEST_true(1)); - C(err, 1, TEST_false(0)); - C(err, 0, TEST_false(1)); + if (!TEST(0, TEST_true(0)) + | !TEST(1, TEST_true(1)) + | !TEST(1, TEST_false(0)) + | !TEST(0, TEST_false(1))) + goto err; return 1; err: @@ -221,14 +235,16 @@ err: static int test_string(void) { static char buf[] = "abc"; - C(err, 1, TEST_str_eq(NULL, NULL)); - C(err, 1, TEST_str_eq("abc", buf)); - C(err, 0, TEST_str_eq("abc", NULL)); - C(err, 0, TEST_str_eq(NULL, buf)); - C(err, 0, TEST_str_ne(NULL, NULL)); - C(err, 0, TEST_str_ne("abc", buf)); - C(err, 1, TEST_str_ne("abc", NULL)); - C(err, 1, TEST_str_ne(NULL, buf)); + + if (!TEST(1, TEST_str_eq(NULL, NULL)) + | !TEST(1, TEST_str_eq("abc", buf)) + | !TEST(0, TEST_str_eq("abc", NULL)) + | !TEST(0, TEST_str_eq(NULL, buf)) + | !TEST(0, TEST_str_ne(NULL, NULL)) + | !TEST(0, TEST_str_ne("abc", buf)) + | !TEST(1, TEST_str_ne("abc", NULL)) + | !TEST(1, TEST_str_ne(NULL, buf))) + goto err; return 1; err: @@ -238,19 +254,30 @@ err: static int test_memory(void) { static char buf[] = "xyz"; - C(err, 1, TEST_mem_eq(NULL, 0, NULL, 0)); - C(err, 1, TEST_mem_eq(NULL, 1, NULL, 2)); - C(err, 0, TEST_mem_eq(NULL, 0, "xyz", 3)); - C(err, 0, TEST_mem_eq(NULL, 0, "", 0)); - C(err, 0, TEST_mem_eq("xyz", 3, NULL, 0)); - C(err, 0, TEST_mem_eq("xyz", 3, buf, sizeof(buf))); - C(err, 1, TEST_mem_eq("xyz", 4, buf, sizeof(buf))); + + if (!TEST(1, TEST_mem_eq(NULL, 0, NULL, 0)) + | !TEST(1, TEST_mem_eq(NULL, 1, NULL, 2)) + | !TEST(0, TEST_mem_eq(NULL, 0, "xyz", 3)) + | !TEST(0, TEST_mem_eq(NULL, 0, "", 0)) + | !TEST(0, TEST_mem_eq("xyz", 3, NULL, 0)) + | !TEST(0, TEST_mem_eq("xyz", 3, buf, sizeof(buf))) + | !TEST(1, TEST_mem_eq("xyz", 4, buf, sizeof(buf)))) + goto err; return 1; err: return 0; } +static int test_memory_overflow(void) +{ + /* Verify that the memory printing overflows without walking the stack */ + const char *p = "1234567890123456789012345678901234567890123456789012"; + const char *q = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"; + + return TEST(0, TEST_mem_eq(p, strlen(p), q, strlen(q))); +} + static int test_messages(void) { TEST_info("This is an %s message.", "info"); @@ -326,6 +353,7 @@ void register_tests(void) ADD_TEST(test_bool); ADD_TEST(test_string); ADD_TEST(test_memory); + ADD_TEST(test_memory_overflow); ADD_TEST(test_messages); ADD_TEST(test_single_eval); } diff --git a/test/testutil/tests.c b/test/testutil/tests.c index 19a366f34c..057ad22b37 100644 --- a/test/testutil/tests.c +++ b/test/testutil/tests.c @@ -15,7 +15,7 @@ #include "../../e_os.h" /* The size of memory buffers to display on failure */ -#define MEM_BUFFER_SIZE (21) +#define MEM_BUFFER_SIZE (33) /* * A common routine to output test failure messages. Generally this should not @@ -267,23 +267,35 @@ int test_strn_ne(const char *file, int line, const char *st1, const char *st2, /* * We could use OPENSSL_buf2hexstr() to do this but trying to allocate memory - * in a failure state isn't generally a great idea. + * in a failure state isn't generally a great idea and if it fails, we want a + * fall back position using caller supplied buffers. + * + * If the return value is different from the buffer supplied, it needs to be + * freed by the caller. */ -static const char *print_mem_maybe_null(const void *s, size_t n, - char out[MEM_BUFFER_SIZE]) +static char *print_mem_maybe_null(const void *s, size_t n, + char outbuf[MEM_BUFFER_SIZE]) { size_t i; const unsigned char *p = (const unsigned char *)s; - int pad = 2*n >= MEM_BUFFER_SIZE; + char *out = outbuf; + int pad = 2 * n >= MEM_BUFFER_SIZE; if (s == NULL) - return "(NULL)"; - if (pad) - n = MEM_BUFFER_SIZE-4; - - for (i=0; i<2*n; i++) { - unsigned char c = (i & 1) != 0 ? p[i / 2] & 15 : p[i / 2] >> 4; - out[i] = "0123456789abcdef"[c]; + return strcpy(outbuf, "(NULL)"); + if (pad) { + if ((out = OPENSSL_malloc(2 * n + 1)) == NULL) { + out = outbuf; + n = (MEM_BUFFER_SIZE - 4) / 2; + } else { + pad = 0; + } + } + + for (i = 0; i < 2 * n; ) { + const unsigned char c = *p++; + out[i++] = "0123456789abcdef"[c >> 4]; + out[i++] = "0123456789abcdef"[c & 15]; } if (pad) { out[i++] = '.'; @@ -291,7 +303,7 @@ static const char *print_mem_maybe_null(const void *s, size_t n, out[i++] = '.'; } out[i] = '\0'; - + return out; } @@ -302,18 +314,17 @@ int test_mem_eq(const char *file, int line, const char *st1, const char *st2, if (s1 == NULL && s2 == NULL) return 1; - if (n1 != n2) { - test_fail_message(NULL, file, line, "memory", - "size mismatch %s %s [%zu] != %s %s [%zu]", - st1, print_mem_maybe_null(s1, n1, b1), n1, - st2, print_mem_maybe_null(s2, n2, b2), n2); - return 0; - } - if (s1 == NULL || s2 == NULL || memcmp(s1, s2, n1) != 0) { + if (n1 != n2 || s1 == NULL || s2 == NULL || memcmp(s1, s2, n1) != 0) { + char *m1 = print_mem_maybe_null(s1, n1, b1); + char *m2 = print_mem_maybe_null(s2, n2, b2); + test_fail_message(NULL, file, line, "memory", - "%s %s [%zu] != %s %s [%zu]", - st1, print_mem_maybe_null(s1, n1, b1), n1, - st2, print_mem_maybe_null(s2, n2, b2), n2); + "%s %s [%zu] == %s %s [%zu]", + st1, m1, n1, st2, m2, n2); + if (m1 != b1) + OPENSSL_free(m1); + if (m2 != b2) + OPENSSL_free(m2); return 0; } return 1; @@ -325,14 +336,20 @@ int test_mem_ne(const char *file, int line, const char *st1, const char *st2, char b1[MEM_BUFFER_SIZE], b2[MEM_BUFFER_SIZE]; if ((s1 == NULL) ^ (s2 == NULL)) - return 1; + return 1; if (n1 != n2) return 1; if (s1 == NULL || memcmp(s1, s2, n1) == 0) { + char *m1 = print_mem_maybe_null(s1, n1, b1); + char *m2 = print_mem_maybe_null(s2, n2, b2); + test_fail_message(NULL, file, line, "memory", "%s %s [%zu] != %s %s [%zu]", - st1, print_mem_maybe_null(s1, n1, b1), n1, - st2, print_mem_maybe_null(s2, n2, b2), n2); + st1, m1, n1, st2, m2, n2); + if (m1 != b1) + OPENSSL_free(m1); + if (m2 != b2) + OPENSSL_free(m2); return 0; } return 1; -- 2.34.1