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

Fix buffer overflow in u8_strlen #197

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

tyler92
Copy link
Contributor

@tyler92 tyler92 commented Oct 18, 2024

This patch fixes a buffer overflow error in the u8_strlen function that happened with invalid UTF-8 input. I added one unit test and it failed without the fix with the following error:

=================================================================
==1789377==ERROR: AddressSanitizer: global-buffer-overflow on address 0x5586e133a5e4 at pc 0x5586e0f09b29 bp 0x7fff5e81a430 sp 0x7fff5e81a420
READ of size 4 at 0x5586e133a5e4 thread T0
    #0 0x5586e0f09b28 in valijson::utils::u8_nextchar(char const*, unsigned long*) valijson/include/valijson/utils/utf8_utils.hpp:38
    #1 0x5586e0f09eaf in valijson::utils::u8_strlen(char const*) valijson/include/valijson/utils/utf8_utils.hpp:50
    #2 0x5586e11bf014 in TestUtf8Utils_Utf8StringLength_Test::TestBody() valijson/tests/test_utf8_utils.cpp:50
    #3 0x5586e125b0af in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) valijson/thirdparty/googletest/googletest/src/gtest.cc:2599
    #4 0x5586e124a4c0 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) valijson/thirdparty/googletest/googletest/src/gtest.cc:2635
    #5 0x5586e11f93fd in testing::Test::Run() valijson/thirdparty/googletest/googletest/src/gtest.cc:2674
    #6 0x5586e11fab16 in testing::TestInfo::Run() valijson/thirdparty/googletest/googletest/src/gtest.cc:2853
    #7 0x5586e11fbd30 in testing::TestSuite::Run() valijson/thirdparty/googletest/googletest/src/gtest.cc:3012
    #8 0x5586e12224dc in testing::internal::UnitTestImpl::RunAllTests() valijson/thirdparty/googletest/googletest/src/gtest.cc:5870
    #9 0x5586e125e471 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) valijson/thirdparty/googletest/googletest/src/gtest.cc:2599
    #10 0x5586e124cea9 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) valijson/thirdparty/googletest/googletest/src/gtest.cc:2635
    #11 0x5586e121ec62 in testing::UnitTest::Run() valijson/thirdparty/googletest/googletest/src/gtest.cc:5444
    #12 0x5586e128687b in RUN_ALL_TESTS() valijson/thirdparty/googletest/googletest/include/gtest/gtest.h:2293
    #13 0x5586e1286744 in main valijson/thirdparty/googletest/googletest/src/gtest_main.cc:51
    #14 0x7f47fd08dd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #15 0x7f47fd08de3f in __libc_start_main_impl ../csu/libc-start.c:392
    #16 0x5586e0e02074 in _start (valijson/build/Desktop/Debug/test_suite+0xa1074)

0x5586e133a5e4 is located 12 bytes to the right of global variable 'offsetsFromUTF8' defined in 'valijson/include/valijson/utils/utf8_utils.hpp:17:23' (0x5586e133a5c0) of size 24
SUMMARY: AddressSanitizer: global-buffer-overflow valijson/include/valijson/utils/utf8_utils.hpp:38 in valijson::utils::u8_nextchar(char const*, unsigned long*)
Shadow bytes around the buggy address:
  0x0ab15c25f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ab15c25f470: 00 00 00 00 06 f9 f9 f9 f9 f9 f9 f9 00 00 00 03
  0x0ab15c25f480: f9 f9 f9 f9 00 00 00 00 05 f9 f9 f9 f9 f9 f9 f9
  0x0ab15c25f490: 00 00 00 00 06 f9 f9 f9 f9 f9 f9 f9 05 f9 f9 f9
  0x0ab15c25f4a0: f9 f9 f9 f9 06 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
=>0x0ab15c25f4b0: 00 00 00 00 00 00 00 00 00 00 00 f9[f9]f9 f9 f9
  0x0ab15c25f4c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ab15c25f4d0: 01 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
  0x0ab15c25f4e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ab15c25f4f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 07 f9
  0x0ab15c25f500: f9 f9 f9 f9 00 04 f9 f9 f9 f9 f9 f9 00 00 00 01
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==1789377==ABORTING

@tyler92 tyler92 mentioned this pull request Oct 20, 2024
@tristanpenman tristanpenman merged commit cc6ca36 into tristanpenman:master Oct 21, 2024
3 checks passed
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

Successfully merging this pull request may close these issues.

2 participants