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

Tests: test-crypto has stack-buffer-overflow when AES encrypting 36 octets #7072

Closed
ADKaster opened this issue May 13, 2021 · 0 comments · Fixed by #7093
Closed

Tests: test-crypto has stack-buffer-overflow when AES encrypting 36 octets #7072

ADKaster opened this issue May 13, 2021 · 0 comments · Fixed by #7093

Comments

@ADKaster
Copy link
Member

ADKaster commented May 13, 2021

The AES CTR 36 octets with 128 bit key | Encrypt test (test-crypto.cpp:832) causes a stack buffer overflow in the in array of 36 bytes:

    {
        // Test Vector #3
        I_TEST((AES CTR 36 octets with 128 bit key | Encrypt))
        u8 key[] {
            0x76, 0x91, 0xbe, 0x03, 0x5e, 0x50, 0x20, 0xa8, 0xac, 0x6e, 0x61, 0x85, 0x29, 0xf9, 0xa0, 0xdc
        };
        u8 ivec[] {
            0x00, 0xe0, 0x01, 0x7b, 0x27, 0x77, 0x7f, 0x3f, 0x4a, 0x17, 0x86, 0xf0, 0x00, 0x00, 0x00, 0x00 + 1 // See CTR.h
        };
        u8 in[] {
            0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23
        };
        u8 out[] {
            0xc1, 0xcf, 0x48, 0xa8, 0x9f, 0x2f, 0xfd, 0xd9, 0xcf, 0x46, 0x52, 0xe9, 0xef, 0xdb, 0x72, 0xd7, 0x45, 0x40, 0xa4, 0x2b, 0xde, 0x6d, 0x78, 0x36, 0xd5, 0x9a, 0x5c, 0xea, 0xae, 0xf3, 0x10, 0x53, 0x25, 0xb2, 0x07, 0x2f
        };
        test_it(AS_BB(key), AS_BB(ivec), AS_BB(in), AS_BB(out));
    }
Test run details with ASAN backtrace
 2/87 Test  #2: Crypto ...............................***Failed    0.07 sec
Testing (AES CBC class name)... PASS 0s 76us
Testing (AES CBC with 128 bit key | Encrypt)... PASS 0s 15us
Testing (AES CBC with 192 bit key | Encrypt)... PASS 0s 7us
Testing (AES CBC with 256 bit key | Encrypt)... PASS 0s 7us
Testing (AES CBC with 256 bit key | Encrypt with unsigned key)... PASS 0s 4us
Testing (AES CTR class name)... PASS 0s 3us
Testing (AES CTR 16 octets with 128 bit key | Encrypt)... PASS 0s 6us
Testing (AES CTR 32 octets with 128 bit key | Encrypt)... PASS 0s 3us
Testing (AES CTR 36 octets with 128 bit key | Encrypt)... =================================================================
==14860==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe06703494 at pc 0x55fb55fb6b6a bp 0x7ffe06702320 sp 0x7ffe06702310
READ of size 1 at 0x7ffe06703494 thread T0
    #0 0x55fb55fb6b69 in Crypto::Cipher::AESCipherBlock::apply_initialization_vector(unsigned char const*) /home/andrew/serenity/Meta/Lagom/../../Userland/Libraries/LibCrypto/Cipher/AES.h:43
    #1 0x55fb55fb6b69 in Crypto::Cipher::CTR<Crypto::Cipher::AESCipher, Crypto::Cipher::IncrementInplace>::encrypt_or_stream(AK::Span<unsigned char const> const*, AK::Span<unsigned char>&, AK::Span<unsigned char const>, AK::Span<unsigned char>*) /home/andrew/serenity/Meta/Lagom/../../Userland/Libraries/LibCrypto/Cipher/Mode/CTR.h:171
    #2 0x55fb55fb7b52 in Crypto::Cipher::CTR<Crypto::Cipher::AESCipher, Crypto::Cipher::IncrementInplace>::encrypt(AK::Span<unsigned char const>, AK::Span<unsigned char>&, AK::Span<unsigned char const>, AK::Span<unsigned char>*) /home/andrew/serenity/Meta/Lagom/../../Userland/Libraries/LibCrypto/Cipher/Mode/CTR.h:118
    #3 0x55fb55f2e89a in operator()<AK::Span<unsigned char const>, AK::Span<unsigned char const>, AK::Span<unsigned char const>, AK::Span<unsigned char const> > /home/andrew/serenity/Userland/Utilities/test-crypto.cpp:783
    #4 0x55fb55f300cc in aes_ctr_test_encrypt /home/andrew/serenity/Userland/Utilities/test-crypto.cpp:845
    #5 0x55fb55f31ce7 in aes_ctr_tests /home/andrew/serenity/Userland/Utilities/test-crypto.cpp:757
    #6 0x55fb55ed808f in main /home/andrew/serenity/Userland/Utilities/test-crypto.cpp:434
    #7 0x7f22a270b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #8 0x55fb55edc60d in _start (/home/andrew/serenity/BuildLagom/test-crypto+0xb7160d)

Address 0x7ffe06703494 is located in stack of thread T0 at offset 2788 in frame
    #0 0x55fb55f2f36f in aes_ctr_test_encrypt /home/andrew/serenity/Userland/Utilities/test-crypto.cpp:777

  This frame has 80 object(s):
    [32, 48) '<unknown>'
    [64, 80) '<unknown>'
    [96, 112) '<unknown>'
    [128, 144) '<unknown>'
    [160, 176) '<unknown>'
    [192, 208) '<unknown>'
    [224, 240) '<unknown>'
    [256, 272) '<unknown>'
    [288, 304) '<unknown>'
    [320, 336) '<unknown>'
    [352, 368) '<unknown>'
    [384, 400) '<unknown>'
    [416, 432) '<unknown>'
    [448, 464) '<unknown>'
    [480, 496) '<unknown>'
    [512, 528) '<unknown>'
    [544, 560) '<unknown>'
    [576, 592) '<unknown>'
    [608, 624) '<unknown>'
    [640, 656) '<unknown>'
    [672, 688) '<unknown>'
    [704, 720) '<unknown>'
    [736, 752) '<unknown>'
    [768, 784) '<unknown>'
    [800, 816) '<unknown>'
    [832, 848) '<unknown>'
    [864, 880) '<unknown>'
    [896, 912) '<unknown>'
    [928, 944) '<unknown>'
    [960, 976) '<unknown>'
    [992, 1008) '<unknown>'
    [1024, 1040) '<unknown>'
    [1056, 1072) '<unknown>'
    [1088, 1104) '<unknown>'
    [1120, 1136) '<unknown>'
    [1152, 1168) '<unknown>'
    [1184, 1200) '<unknown>'
    [1216, 1232) '<unknown>'
    [1248, 1264) '<unknown>'
    [1280, 1296) '<unknown>'
    [1312, 1328) 'key' (line 798)
    [1344, 1360) 'ivec' (line 801)
    [1376, 1392) 'in' (line 804)
    [1408, 1424) 'out' (line 807)
    [1440, 1456) 'key' (line 815)
    [1472, 1488) 'ivec' (line 818)
    [1504, 1520) 'key' (line 833)
    [1536, 1552) 'ivec' (line 836)
    [1568, 1584) 'ivec' (line 853)
    [1600, 1616) 'in' (line 856)
    [1632, 1648) 'out' (line 859)
    [1664, 1680) 'ivec' (line 870)
    [1696, 1712) 'ivec' (line 887)
    [1728, 1744) 'ivec' (line 904)
    [1760, 1776) 'in' (line 907)
    [1792, 1808) 'out' (line 910)
    [1824, 1840) 'ivec' (line 921)
    [1856, 1872) 'ivec' (line 938)
    [1888, 1904) 'ivec' (line 956)
    [1920, 1944) 'key' (line 850)
    [1984, 2008) 'key' (line 867)
    [2048, 2072) 'key' (line 884)
    [2112, 2144) 'in' (line 821)
    [2176, 2208) 'out' (line 824)
    [2240, 2272) 'in' (line 873)
    [2304, 2336) 'out' (line 876)
    [2368, 2400) 'key' (line 901)
    [2432, 2464) 'key' (line 918)
    [2496, 2528) 'in' (line 924)
    [2560, 2592) 'out' (line 927)
    [2624, 2656) 'key' (line 935)
    [2688, 2720) 'key' (line 953)
    [2752, 2788) 'in' (line 839) <== Memory access at offset 2788 overflows this variable
    [2832, 2868) 'out' (line 842)
    [2912, 2948) 'in' (line 890)
    [2992, 3028) 'out' (line 893)
    [3072, 3108) 'in' (line 941)
    [3152, 3188) 'out' (line 944)
    [3232, 3268) 'in' (line 959)
    [3312, 3348) 'out' (line 962)
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 /home/andrew/serenity/Meta/Lagom/../../Userland/Libraries/LibCrypto/Cipher/AES.h:43 in Crypto::Cipher::AESCipherBlock::apply_initialization_vector(unsigned char const*)
Shadow bytes around the buggy address:
  0x100040cd8640: f8 f8 f2 f2 f2 f2 f8 f8 f8 f8 f2 f2 f2 f2 00 00
  0x100040cd8650: 00 00 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00
  0x100040cd8660: 00 00 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00
  0x100040cd8670: 00 00 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00
  0x100040cd8680: 00 00 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00
=>0x100040cd8690: 00 00[04]f2 f2 f2 f2 f2 00 00 00 00 04 f2 f2 f2
  0x100040cd86a0: f2 f2 00 00 00 00 04 f2 f2 f2 f2 f2 00 00 00 00
  0x100040cd86b0: 04 f2 f2 f2 f2 f2 00 00 00 00 04 f2 f2 f2 f2 f2
  0x100040cd86c0: 00 00 00 00 04 f2 f2 f2 f2 f2 00 00 00 00 04 f2
  0x100040cd86d0: f2 f2 f2 f2 00 00 00 00 04 f3 f3 f3 f3 f3 00 00
  0x100040cd86e0: 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
  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
==14860==ABORTING
alimpfard added a commit to alimpfard/serenity that referenced this issue May 14, 2021
Just take ReadonlyBytes instead of a raw pointer.
Fixes SerenityOS#7072 (tested with the ASAN build fixed by SerenityOS#7060).
linusg pushed a commit that referenced this issue May 14, 2021
Just take ReadonlyBytes instead of a raw pointer.
Fixes #7072 (tested with the ASAN build fixed by #7060).
Hendiadyoin1 pushed a commit to Hendiadyoin1/serenity that referenced this issue May 24, 2021
Just take ReadonlyBytes instead of a raw pointer.
Fixes SerenityOS#7072 (tested with the ASAN build fixed by SerenityOS#7060).
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 a pull request may close this issue.

1 participant