Skip to content

Added Unicode Tests including OOB Buffer Tests #3367

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from
Draft

Added Unicode Tests including OOB Buffer Tests #3367

wants to merge 9 commits into from

Conversation

@Taha-Ebrahim
Copy link
Contributor

@Taha-Ebrahim Taha-Ebrahim commented Aug 19, 2025

This PR attempts to test the sanitizer crashes around UTF-16 to UTF-8 conversion.

The current #3199 does not seem to resolve the issue based on my testing. I still get an address sanitizer issue.

This PR contains all of the changes that were made in #3199 and also contains the new test_tsk_unicode.cpp and corresponding updates to Makefile.am.

FAIL: test/catch_runner
=======================

=================================================================
==6121==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x00016d3ae160 at pc 0x0001031553ac bp 0x00016d3ae0d0 sp 0x00016d3ae0c8
READ of size 8 at 0x00016d3ae160 thread T0
    #0 0x0001031553a8 in tsk_UTF16toUTF8 tsk_unicode.c:152
    #1 0x000102a5a990 in ____C_A_T_C_H____T_E_S_T____29() test_tsk_unicode.cpp:269
    #2 0x000102aefb24 in Catch::RunContext::runCurrentTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&) catch.hpp:12989
    #3 0x000102aef208 in Catch::RunContext::runTest(Catch::TestCase const&) catch.hpp:12750
    #4 0x000102af4820 in Catch::Session::runInternal() catch.hpp:13549
    #5 0x000102af3da8 in Catch::Session::run() catch.hpp:13505
    #6 0x000102b0a3ec in main catch.hpp:17504
    #7 0x0001930f6b94 in start+0x17b8 (dyld:arm64e+0xfffffffffff3ab94)

Address 0x00016d3ae160 is located in stack of thread T0 at offset 64 in frame
    #0 0x000102a5a680 in ____C_A_T_C_H____T_E_S_T____29() test_tsk_unicode.cpp:247

  This frame has 19 object(s):
    [32, 48) 'agg.tmp.i'
    [64, 68) 'source_buffer' (line 251) <== Memory access at offset 64 partially overflows this variable
    [80, 90) 'target_buffer' (line 258)
    [112, 120) 'target_ptr' (line 259)
    [144, 280) 'ref.tmp' (line 268)
    [352, 416) 'ref.tmp5' (line 268)
    [448, 464) 'ref.tmp6' (line 268)
    [480, 504) 'ref.tmp7' (line 268)
    [544, 616) 'catchAssertionHandler' (line 269)
    [656, 672) 'ref.tmp23' (line 269)
    [688, 704) 'ref.tmp25' (line 269)
    [720, 736) 'agg.tmp' (line 269)
    [752, 824) 'catchAssertionHandler50' (line 275)
    [864, 880) 'ref.tmp51' (line 275)
    [896, 912) 'ref.tmp53' (line 275)
    [928, 944) 'agg.tmp55' (line 275)
    [960, 1008) 'ref.tmp62' (line 275)
    [1040, 1044) 'ref.tmp65' (line 275)
    [1056, 1060) 'ref.tmp72' (line 275)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow tsk_unicode.c:152 in tsk_UTF16toUTF8
Shadow bytes around the buggy address:
  0x00016d3ade80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00016d3adf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00016d3adf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00016d3ae000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00016d3ae080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x00016d3ae100: 00 00 00 00 f1 f1 f1 f1 f8 f8 f2 f2[04]f2 00 02
  0x00016d3ae180: f2 f2 00 f2 f2 f2 00 00 00 00 00 00 00 00 00 00
  0x00016d3ae200: 00 00 00 00 00 00 00 f2 f2 f2 f2 f2 f2 f2 f2 f2
  0x00016d3ae280: f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2 f8 f8 f2 f2
  0x00016d3ae300: f8 f8 f8 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00
  0x00016d3ae380: 00 f2 f2 f2 f2 f2 f8 f8 f2 f2 f8 f8 f2 f2 f8 f8
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
==6121==ABORTING

@Taha-Ebrahim Taha-Ebrahim mentioned this pull request Aug 19, 2025
Open
@codecov
Copy link

codecov bot commented Aug 19, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 25.01%. Comparing base (e084d90) to head (0af0242).
⚠️ Report is 108 commits behind head on develop.

Files with missing lines Patch % Lines
tsk/base/tsk_unicode.c 88.89% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #3367      +/-   ##
===========================================
+ Coverage    24.00%   25.01%   +1.02%     
===========================================
  Files          190      190              
  Lines        42542    42549       +7     
  Branches      7164     7165       +1     
===========================================
+ Hits         10209    10643     +434     
+ Misses       32333    31906     -427
Flag Coverage Δ
unittests 25.01% <93.33%> (+1.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
tsk/fs/ntfs_dent.cpp 45.94% <100.00%> (-0.16%) ⬇️
tsk/base/tsk_unicode.c 67.07% <88.89%> (+56.73%) ⬆️

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@simsong
Copy link
Member

simsong commented Aug 19, 2025

@joachimmetz — Here we now have a test that fails with your patch. Please take a look when you have a chance.

@simsong simsong marked this pull request as draft August 19, 2025 12:07
@joachimmetz
Copy link
Contributor

joachimmetz commented Sep 5, 2025

@simsong want me to integrate these changes into #3199 ? Or want to submit this first with a failing test and then merge #3199 on top of this to see if the test passes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

failing test has test case

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Taha-Ebrahim @simsong @joachimmetz