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

Stack buffer overflow in v043 #94

Closed
drach opened this Issue Dec 10, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@drach

drach commented Dec 10, 2015

VS2013 and Xcode711/Asan detect a stack buffer overflow in a released v0.4.3 with a specific input data.

How to repro

Use the following PVRTC4 compressed sample image with a fullbench app, called without additional parameters.
https://www.dropbox.com/s/tlgr7lxpmtiq4yw/sample.pvr?dl=0

Output with VS2013-update5, Windows7:
*** Zstandard speed analyzer  32-bits, by Yann Collet (Dec 10 2015) ***
 D:\work\zstd\release-v043\visual\2012\sample.pvr :
 1- ZSTD_compress                  :     3.4 MB/s  (  2120226)
11- ZSTD_decompress                :     7.4 MB/s  (  2796340)
31- ZSTD_decodeLiteralsBlock       :    11.8 MB/s  (    74273)
 1- ZSTD_decodeSeqHeaders          :


Run-Time Check Failure #2 - Stack around the variable 'DTableOffb' was corrupted.


>   fullbench.exe!local_ZSTD_decodeSeqHeaders(void * dst, unsigned int dstSize, void * buff2, const void * src, unsigned int srcSize) Line 244  C
    fullbench.exe!benchMem(void * src, unsigned int srcSize, unsigned int benchNb) Line 358 C
    fullbench.exe!benchFiles(char * * fileNamesTable, int nbFiles, unsigned int benchNb) Line 468   C
    fullbench.exe!main(int argc, char * * argv) Line 584    C
    [External Code] 
    [Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]  
Output with Xcode711, Clang700.1.76, enabled ASan, iOS9.1:
AddressSanitizer debugger support is active. Memory error breakpoint has been installed and you can now use the 'memory history' command.
2015-12-10 12:48:24.980 TestLZ4[2018:627417] Started
*** Zstandard speed analyzer  64-bits, by Yann Collet (Dec 10 2015) ***
Loading /var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/sample.pvr...       


 /var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/sample.pvr : 
 1- ZSTD_compress                  : 
 1- ZSTD_compress                  :     4.1 MB/s  (  2120226)
 2- ZSTD_compress                  : 
 2- ZSTD_compress                  :     4.1 MB/s  (  2120226)
 3- ZSTD_compress                  : 
 3- ZSTD_compress                  :     4.1 MB/s  (  2120226)
 4- ZSTD_compress                  : 
 4- ZSTD_compress                  :     4.1 MB/s  (  2120226)
 5- ZSTD_compress                  : 
 5- ZSTD_compress                  :     4.1 MB/s  (  2120226)
 6- ZSTD_compress                  : 
 6- ZSTD_compress                  :     4.1 MB/s  (  2120226)
 1- ZSTD_compress                  :     4.1 MB/s  (  2120226)
 1- ZSTD_decompress                : 
 1- ZSTD_decompress                :    15.0 MB/s  (  2796340)
 2- ZSTD_decompress                : 
 2- ZSTD_decompress                :    15.0 MB/s  (  2796340)
 3- ZSTD_decompress                : 
 3- ZSTD_decompress                :    15.0 MB/s  (  2796340)
 4- ZSTD_decompress                : 
 4- ZSTD_decompress                :    15.0 MB/s  (  2796340)
 5- ZSTD_decompress                : 
 5- ZSTD_decompress                :    15.0 MB/s  (  2796340)
 6- ZSTD_decompress                : 
 6- ZSTD_decompress                :    15.0 MB/s  (  2796340)
11- ZSTD_decompress                :    15.0 MB/s  (  2796340)
 1- ZSTD_decodeLiteralsBlock       : 
 1- ZSTD_decodeLiteralsBlock       :    26.1 MB/s  (    74273)
 2- ZSTD_decodeLiteralsBlock       : 
 2- ZSTD_decodeLiteralsBlock       :    26.1 MB/s  (    74273)
 3- ZSTD_decodeLiteralsBlock       : 
 3- ZSTD_decodeLiteralsBlock       :    26.1 MB/s  (    74273)
 4- ZSTD_decodeLiteralsBlock       : 
 4- ZSTD_decodeLiteralsBlock       :    26.1 MB/s  (    74273)
 5- ZSTD_decodeLiteralsBlock       : 
 5- ZSTD_decodeLiteralsBlock       :    26.1 MB/s  (    74273)
 6- ZSTD_decodeLiteralsBlock       : 
 6- ZSTD_decodeLiteralsBlock       :    26.1 MB/s  (    74273)
31- ZSTD_decodeLiteralsBlock       :    26.1 MB/s  (    74273)
 1- ZSTD_decodeSeqHeaders          : 
=================================================================
==2018==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x00016fd84ee2 at pc 0x0001000c29ec bp 0x00016fd81430 sp 0x00016fd81428
WRITE of size 1 at 0x00016fd84ee2 thread T0
    #0 0x1000c29eb in FSE_buildDTable (/var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/TestLZ4+0x10004a9eb)
    #1 0x10009ff7f in ZSTD_decodeSeqHeaders (/var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/TestLZ4+0x100027f7f)
    #2 0x1000bcdaf in local_ZSTD_decodeSeqHeaders (/var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/TestLZ4+0x100044daf)
    #3 0x1000bd683 in benchMem (/var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/TestLZ4+0x100045683)
    #4 0x1000be247 in benchFiles (/var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/TestLZ4+0x100046247)
    #5 0x1000bee27 in zstd_start_benchmark (/var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/TestLZ4+0x100046e27)
    #6 0x10009d15b in -[ViewController doAsyncTestButton:] (/var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/TestLZ4+0x10002515b)
    #7 0x189ca7cfb in <redacted> (/System/Library/Frameworks/UIKit.framework/UIKit+0x4fcfb)
    #8 0x189ca7c77 in <redacted> (/System/Library/Frameworks/UIKit.framework/UIKit+0x4fc77)
    #9 0x189c8f92f in <redacted> (/System/Library/Frameworks/UIKit.framework/UIKit+0x3792f)
    #10 0x189cb03cb in <redacted> (/System/Library/Frameworks/UIKit.framework/UIKit+0x583cb)
    #11 0x189ca7013 in <redacted> (/System/Library/Frameworks/UIKit.framework/UIKit+0x4f013)
    #12 0x189c9fcdb in <redacted> (/System/Library/Frameworks/UIKit.framework/UIKit+0x47cdb)
    #13 0x189c704a3 in <redacted> (/System/Library/Frameworks/UIKit.framework/UIKit+0x184a3)
    #14 0x189c6e76b in <redacted> (/System/Library/Frameworks/UIKit.framework/UIKit+0x1676b)
    #15 0x184694543 in <redacted> (/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation+0xdc543)
    #16 0x184693fd7 in <redacted> (/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation+0xdbfd7)
    #17 0x184691cd7 in <redacted> (/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation+0xd9cd7)
    #18 0x1845c0c9f in CFRunLoopRunSpecific (/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation+0x8c9f)
    #19 0x18f7fc087 in GSEventRunModal (/System/Library/PrivateFrameworks/GraphicsServices.framework/GraphicsServices+0xc087)
    #20 0x189cd8ffb in UIApplicationMain (/System/Library/Frameworks/UIKit.framework/UIKit+0x80ffb)
    #21 0x1000f724f in main (/var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/TestLZ4+0x10007f24f)
    #22 0x199ade8b7 in <redacted> (/usr/lib/system/libdyld.dylib+0x28b7)

Address 0x00016fd84ee2 is located in stack of thread T0 at offset 12578 in frame
    #0 0x1000bcb27 in local_ZSTD_decodeSeqHeaders (/var/mobile/Containers/Bundle/Application/1FA3B3BC-B5AD-486D-BAD2-144B631187D1/TestLZ4.app/TestLZ4+0x100044b27)

  This frame has 6 object(s):
    [32, 8224) 'DTableML'
    [8480, 12576) 'DTableLL' <== Memory access at offset 12578 overflows this variable
    [12704, 14752) 'DTableOffb'
    [14880, 14888) 'dumps'
    [14912, 14920) 'length'
    [14944, 14948) 'nbSeq'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow ??:0 FSE_buildDTable
Shadow bytes around the buggy address:
  0x00014e1b0980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00014e1b0990: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00014e1b09a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00014e1b09b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00014e1b09c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x00014e1b09d0: 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2 f2
  0x00014e1b09e0: f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 f2 00 00 00 00
  0x00014e1b09f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00014e1b0a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00014e1b0a10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00014e1b0a20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  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
==2018==ABORTING
AddressSanitizer report breakpoint hit. Use 'thread info -s' to get extended information about the report.

(lldb) bt
* thread #1: tid = 0x992d9, 0x00000001001650c4 libclang_rt.asan_ios_dynamic.dylib`__asan::AsanDie(), queue = 'com.apple.main-thread', stop reason = Stack buffer overflow detected
    frame #0: 0x00000001001650c4 libclang_rt.asan_ios_dynamic.dylib`__asan::AsanDie()
    frame #1: 0x0000000100168b80 libclang_rt.asan_ios_dynamic.dylib`__sanitizer::Die() + 44
    frame #2: 0x0000000100163ed4 libclang_rt.asan_ios_dynamic.dylib`__asan::ScopedInErrorReport::~ScopedInErrorReport() + 336
    frame #3: 0x0000000100163c6c libclang_rt.asan_ios_dynamic.dylib`__asan::ScopedInErrorReport::~ScopedInErrorReport() + 12
    frame #4: 0x00000001001637e8 libclang_rt.asan_ios_dynamic.dylib`__asan_report_error + 3216
    frame #5: 0x00000001001641f8 libclang_rt.asan_ios_dynamic.dylib`__asan_report_store1 + 44
  * frame #6: 0x00000001000c29ec TestLZ4`FSE_buildDTable(dt=0x000000016fd83ee0, normalizedCounter=0x000000016fd818f0, maxSymbolValue=63, tableLog=10) + 856 at fse.c:373
    frame #7: 0x000000010009ff80 TestLZ4`ZSTD_decodeSeqHeaders(nbSeq=0x000000016fd85820, dumpsPtr=0x000000016fd857e0, dumpsLengthPtr=0x000000016fd85800, DTableLL=0x000000016fd83ee0, DTableML=0x000000016fd81de0, DTableOffb=0x000000016fd84f60, src=0x000000010a574800, srcSize=11133) + 3100 at zstd_decompress.c:377
    frame #8: 0x00000001000bcdb0 TestLZ4`local_ZSTD_decodeSeqHeaders(dst=0x000000010a2bc800, dstSize=2818710, buff2=0x000000010a574800, src=0x0000000108404800, srcSize=131072) + 664 at zstd_fullbench.c:243
    frame #9: 0x00000001000bd684 TestLZ4`benchMem(src=0x0000000108404800, srcSize=131072, benchNb=32) + 2116 at zstd_fullbench.c:358
    frame #10: 0x00000001000be248 TestLZ4`benchFiles(fileNamesTable=0x000000016fd85e98, nbFiles=1, benchNb=32) + 1100 at zstd_fullbench.c:468
    frame #11: 0x00000001000bee28 TestLZ4`zstd_start_benchmark(argc=2, argv=0x000000016fd85e90) + 2188 at zstd_fullbench.c:584
    frame #12: 0x000000010009d15c TestLZ4`-[ViewController doAsyncTestButton:](self=0x0000000107507880, _cmd="doAsyncTestButton:", sender=<unavailable>) + 792 at ViewController.mm:55
    frame #13: 0x0000000189ca7cfc UIKit`-[UIApplication sendAction:to:from:forEvent:] + 100
    frame #14: 0x0000000189ca7c78 UIKit`-[UIControl sendAction:to:forEvent:] + 80
    frame #15: 0x0000000189c8f930 UIKit`-[UIControl _sendActionsForEvents:withEvent:] + 416
    frame #16: 0x0000000189cb03cc UIKit`-[UIControl touchesBegan:withEvent:] + 268
    frame #17: 0x0000000189ca7014 UIKit`-[UIWindow _sendTouchesForEvent:] + 376
    frame #18: 0x0000000189c9fcdc UIKit`-[UIWindow sendEvent:] + 784
    frame #19: 0x0000000189c704a4 UIKit`-[UIApplication sendEvent:] + 248
    frame #20: 0x0000000189c6e76c UIKit`_UIApplicationHandleEventQueue + 5528
    frame #21: 0x0000000184694544 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 24
    frame #22: 0x0000000184693fd8 CoreFoundation`__CFRunLoopDoSources0 + 540
    frame #23: 0x0000000184691cd8 CoreFoundation`__CFRunLoopRun + 724
    frame #24: 0x00000001845c0ca0 CoreFoundation`CFRunLoopRunSpecific + 384
    frame #25: 0x000000018f7fc088 GraphicsServices`GSEventRunModal + 180
    frame #26: 0x0000000189cd8ffc UIKit`UIApplicationMain + 204
    frame #27: 0x00000001000f7250 TestLZ4`main(argc=1, argv=0x000000016fd87a90) + 124 at main.m:16
    frame #28: 0x0000000199ade8b8 libdyld.dylib`start + 4
(lldb) 
Possible consequences

In an existing quite a big iOS app the invocation of ZSTD_decompress (exactly this function, not its internals) may just crash after several dozens of successful calls, it looks like a problem in a corrupted stack. I didn't manage to localize a problem in a separated sample, only found an issue with a fullbench above. Still not totally sure whether both issues have the same root, but it may be.

@Cyan4973

This comment has been minimized.

Show comment
Hide comment
@Cyan4973

Cyan4973 Dec 10, 2015

Contributor

Thanks for reporting @drach. I'll look into it.

There is also a chance that the problem lies into fullbench rather than zstd.
Since you provide the sample to reproduce the issue, investigation should find it.

Contributor

Cyan4973 commented Dec 10, 2015

Thanks for reporting @drach. I'll look into it.

There is also a chance that the problem lies into fullbench rather than zstd.
Since you provide the sample to reproduce the issue, investigation should find it.

Cyan4973 added a commit that referenced this issue Dec 10, 2015

@Cyan4973

This comment has been minimized.

Show comment
Hide comment
@Cyan4973

Cyan4973 Dec 10, 2015

Contributor

I believe to have fixed this issue within latest update of "dev" branch 695a6cb.

The issue was within fullbench, zstd library looks unaffected (by this bug).
fullbench improperly allocated space for the ZSTD_decodeSeqHeaders test, which was extended by one cell in recent versions of FSE.

Since the issue is not specific to your sample, I'm surprised it has not show up earlier within Continuous Integration tests, as fullbench is tested with asan enabled. I suspect FSE dynamically downsizes reference sample statistics as they are generated from a small sample. Side effect : it doesn't test the limits of internal buffers.

If you witness some instability issues outside of fullbench, it may be related to something else.

Contributor

Cyan4973 commented Dec 10, 2015

I believe to have fixed this issue within latest update of "dev" branch 695a6cb.

The issue was within fullbench, zstd library looks unaffected (by this bug).
fullbench improperly allocated space for the ZSTD_decodeSeqHeaders test, which was extended by one cell in recent versions of FSE.

Since the issue is not specific to your sample, I'm surprised it has not show up earlier within Continuous Integration tests, as fullbench is tested with asan enabled. I suspect FSE dynamically downsizes reference sample statistics as they are generated from a small sample. Side effect : it doesn't test the limits of internal buffers.

If you witness some instability issues outside of fullbench, it may be related to something else.

@drach

This comment has been minimized.

Show comment
Hide comment
@drach

drach Dec 10, 2015

Indeed, fullbench is fixed in "dev", thank you.

If you witness some instability issues outside of fullbench, it may be related to something else.

Agree, will check it closer.

drach commented Dec 10, 2015

Indeed, fullbench is fixed in "dev", thank you.

If you witness some instability issues outside of fullbench, it may be related to something else.

Agree, will check it closer.

@drach drach closed this Dec 10, 2015

@drach

This comment has been minimized.

Show comment
Hide comment
@drach

drach Dec 14, 2015

In an existing quite a big iOS app the invocation of ZSTD_decompress (exactly this function, not its internals) may just crash after several dozens of successful calls, it looks like a problem in a corrupted stack.

I was wrong. We call ZSTD_decompress in different threads, some of them are set up with a 128Kb stack size that turns to be small for a ZSTD context. I was careless to notice it at first because the same code and data worked normally with LZ4 and Zlib's DEFLATE.

drach commented Dec 14, 2015

In an existing quite a big iOS app the invocation of ZSTD_decompress (exactly this function, not its internals) may just crash after several dozens of successful calls, it looks like a problem in a corrupted stack.

I was wrong. We call ZSTD_decompress in different threads, some of them are set up with a 128Kb stack size that turns to be small for a ZSTD context. I was careless to notice it at first because the same code and data worked normally with LZ4 and Zlib's DEFLATE.

@Cyan4973

This comment has been minimized.

Show comment
Hide comment
@Cyan4973

Cyan4973 Dec 14, 2015

Contributor

Thanks for feedback @drach.

Which reminds me : the decompression context is quite large currently, and it's part of my todo list to tame it.

In the meantime, should you need to keep your stack at 128K per thread, I suggest creating a context per thread with ZSTD_createDCtx(), and use ZSTD_decompressDCtx() (which is not yet exposed unfortunately, but present within zstd_decompress.c). It will avoid usage of the stack.

Contributor

Cyan4973 commented Dec 14, 2015

Thanks for feedback @drach.

Which reminds me : the decompression context is quite large currently, and it's part of my todo list to tame it.

In the meantime, should you need to keep your stack at 128K per thread, I suggest creating a context per thread with ZSTD_createDCtx(), and use ZSTD_decompressDCtx() (which is not yet exposed unfortunately, but present within zstd_decompress.c). It will avoid usage of the stack.

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