From 16e185c9081530859c17270fbaf5798f0ea871f8 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Tue, 10 Sep 2013 16:56:34 -0700 Subject: [PATCH] __memcpy_chk: Fix signed cmp of unsigned values. I accidentally did a signed comparison of the size_t values passed in for three of the _chk functions. Changing them to unsigned compares. Add three new tests to verify this failure is fixed. Bug: 10691831 Merge from internal master. (cherry-picked from 883ef2499c2ff76605f73b1240f719ca6282e554) Change-Id: Id9a96b549435f5d9b61dc132cf1082e0e30889f5 --- .../arch-arm/cortex-a15/bionic/__strcat_chk.S | 2 +- .../arch-arm/cortex-a15/bionic/__strcpy_chk.S | 2 +- libc/arch-arm/cortex-a15/bionic/memcpy.S | 2 +- libc/arch-arm/cortex-a9/bionic/__strcat_chk.S | 2 +- libc/arch-arm/cortex-a9/bionic/__strcpy_chk.S | 2 +- libc/arch-arm/cortex-a9/bionic/memcpy.S | 2 +- libc/arch-arm/krait/bionic/__strcat_chk.S | 2 +- libc/arch-arm/krait/bionic/__strcpy_chk.S | 2 +- libc/arch-arm/krait/bionic/memcpy.S | 2 +- tests/fortify_test.cpp | 55 +++++++++++++++++++ 10 files changed, 64 insertions(+), 9 deletions(-) diff --git a/libc/arch-arm/cortex-a15/bionic/__strcat_chk.S b/libc/arch-arm/cortex-a15/bionic/__strcat_chk.S index 46936005d..4aaa9f1f8 100644 --- a/libc/arch-arm/cortex-a15/bionic/__strcat_chk.S +++ b/libc/arch-arm/cortex-a15/bionic/__strcat_chk.S @@ -180,7 +180,7 @@ ENTRY(__strcat_chk) .L_strlen_done: add r2, r3, r4 cmp r2, lr - bgt __strcat_chk_failed + bhi __strcat_chk_failed // Set up the registers for the memcpy code. mov r1, r5 diff --git a/libc/arch-arm/cortex-a15/bionic/__strcpy_chk.S b/libc/arch-arm/cortex-a15/bionic/__strcpy_chk.S index 1224b491d..05152e63d 100644 --- a/libc/arch-arm/cortex-a15/bionic/__strcpy_chk.S +++ b/libc/arch-arm/cortex-a15/bionic/__strcpy_chk.S @@ -151,7 +151,7 @@ ENTRY(__strcpy_chk) pld [r1, #64] ldr r0, [sp] cmp r3, lr - bge __strcpy_chk_failed + bhs __strcpy_chk_failed // Add 1 for copy length to get the string terminator. add r2, r3, #1 diff --git a/libc/arch-arm/cortex-a15/bionic/memcpy.S b/libc/arch-arm/cortex-a15/bionic/memcpy.S index a300e43db..a8432306e 100644 --- a/libc/arch-arm/cortex-a15/bionic/memcpy.S +++ b/libc/arch-arm/cortex-a15/bionic/memcpy.S @@ -65,7 +65,7 @@ ENTRY(__memcpy_chk) .cfi_startproc cmp r2, r3 - bgt __memcpy_chk_fail + bhi __memcpy_chk_fail // Fall through to memcpy... .cfi_endproc diff --git a/libc/arch-arm/cortex-a9/bionic/__strcat_chk.S b/libc/arch-arm/cortex-a9/bionic/__strcat_chk.S index cc43456c3..78cf19a2e 100644 --- a/libc/arch-arm/cortex-a9/bionic/__strcat_chk.S +++ b/libc/arch-arm/cortex-a9/bionic/__strcat_chk.S @@ -183,7 +183,7 @@ ENTRY(__strcat_chk) .L_strlen_done: add r2, r3, r4 cmp r2, lr - bgt __strcat_chk_fail + bhi __strcat_chk_fail // Set up the registers for the memcpy code. mov r1, r5 diff --git a/libc/arch-arm/cortex-a9/bionic/__strcpy_chk.S b/libc/arch-arm/cortex-a9/bionic/__strcpy_chk.S index dd3370bae..d0acf1e4f 100644 --- a/libc/arch-arm/cortex-a9/bionic/__strcpy_chk.S +++ b/libc/arch-arm/cortex-a9/bionic/__strcpy_chk.S @@ -153,7 +153,7 @@ ENTRY(__strcpy_chk) pld [r1, #64] ldr r0, [sp] cmp r3, lr - bge __strcpy_chk_fail + bhs __strcpy_chk_fail // Add 1 for copy length to get the string terminator. add r2, r3, #1 diff --git a/libc/arch-arm/cortex-a9/bionic/memcpy.S b/libc/arch-arm/cortex-a9/bionic/memcpy.S index 21e0ebea6..5c4c42821 100644 --- a/libc/arch-arm/cortex-a9/bionic/memcpy.S +++ b/libc/arch-arm/cortex-a9/bionic/memcpy.S @@ -43,7 +43,7 @@ ENTRY(__memcpy_chk) .cfi_startproc cmp r2, r3 - bgt __memcpy_chk_fail + bhi __memcpy_chk_fail // Fall through to memcpy... .cfi_endproc diff --git a/libc/arch-arm/krait/bionic/__strcat_chk.S b/libc/arch-arm/krait/bionic/__strcat_chk.S index ec99077c3..956b4615a 100644 --- a/libc/arch-arm/krait/bionic/__strcat_chk.S +++ b/libc/arch-arm/krait/bionic/__strcat_chk.S @@ -180,7 +180,7 @@ ENTRY(__strcat_chk) .L_strlen_done: add r2, r3, r4 cmp r2, lr - bgt __strcat_chk_failed + bhi __strcat_chk_failed // Set up the registers for the memcpy code. mov r1, r5 diff --git a/libc/arch-arm/krait/bionic/__strcpy_chk.S b/libc/arch-arm/krait/bionic/__strcpy_chk.S index 7da4d1536..402cac678 100644 --- a/libc/arch-arm/krait/bionic/__strcpy_chk.S +++ b/libc/arch-arm/krait/bionic/__strcpy_chk.S @@ -151,7 +151,7 @@ ENTRY(__strcpy_chk) pld [r1, #64] ldr r0, [sp] cmp r3, lr - bge __strcpy_chk_failed + bhs __strcpy_chk_failed // Add 1 for copy length to get the string terminator. add r2, r3, #1 diff --git a/libc/arch-arm/krait/bionic/memcpy.S b/libc/arch-arm/krait/bionic/memcpy.S index e433402c9..ea040bcee 100644 --- a/libc/arch-arm/krait/bionic/memcpy.S +++ b/libc/arch-arm/krait/bionic/memcpy.S @@ -46,7 +46,7 @@ ENTRY(__memcpy_chk) .cfi_startproc cmp r2, r3 - bgt __memcpy_chk_fail + bhi __memcpy_chk_fail // Fall through to memcpy... .cfi_endproc diff --git a/tests/fortify_test.cpp b/tests/fortify_test.cpp index aa1373617..5ec15b811 100644 --- a/tests/fortify_test.cpp +++ b/tests/fortify_test.cpp @@ -717,3 +717,58 @@ TEST(TEST_NAME, strncpy2) { ASSERT_EQ('\0', dst[13]); ASSERT_EQ('\0', dst[14]); } + +TEST(TEST_NAME, strcat_chk_max_int_size) { + char buf[10]; + memset(buf, 'A', sizeof(buf)); + buf[0] = 'a'; + buf[1] = '\0'; + char* res = __strcat_chk(buf, "01234567", (size_t)-1); + ASSERT_EQ(buf, res); + ASSERT_EQ('a', buf[0]); + ASSERT_EQ('0', buf[1]); + ASSERT_EQ('1', buf[2]); + ASSERT_EQ('2', buf[3]); + ASSERT_EQ('3', buf[4]); + ASSERT_EQ('4', buf[5]); + ASSERT_EQ('5', buf[6]); + ASSERT_EQ('6', buf[7]); + ASSERT_EQ('7', buf[8]); + ASSERT_EQ('\0', buf[9]); +} + +extern "C" char* __strcpy_chk(char*, const char*, size_t); + +TEST(TEST_NAME, strcpy_chk_max_int_size) { + char buf[10]; + char* res = __strcpy_chk(buf, "012345678", (size_t)-1); + ASSERT_EQ(buf, res); + ASSERT_EQ('0', buf[0]); + ASSERT_EQ('1', buf[1]); + ASSERT_EQ('2', buf[2]); + ASSERT_EQ('3', buf[3]); + ASSERT_EQ('4', buf[4]); + ASSERT_EQ('5', buf[5]); + ASSERT_EQ('6', buf[6]); + ASSERT_EQ('7', buf[7]); + ASSERT_EQ('8', buf[8]); + ASSERT_EQ('\0', buf[9]); +} + +extern "C" void* __memcpy_chk(void*, const void*, size_t, size_t); + +TEST(TEST_NAME, memcpy_chk_max_int_size) { + char buf[10]; + void* res = __memcpy_chk(buf, "012345678", sizeof(buf), (size_t)-1); + ASSERT_EQ((void*)buf, res); + ASSERT_EQ('0', buf[0]); + ASSERT_EQ('1', buf[1]); + ASSERT_EQ('2', buf[2]); + ASSERT_EQ('3', buf[3]); + ASSERT_EQ('4', buf[4]); + ASSERT_EQ('5', buf[5]); + ASSERT_EQ('6', buf[6]); + ASSERT_EQ('7', buf[7]); + ASSERT_EQ('8', buf[8]); + ASSERT_EQ('\0', buf[9]); +}