Memory Corruption in FixedKeyHashing

[lenerd]

Using AddressSanitizer with the SHA1 example from the ABY I found a buffer overflow on the heap.

GDB stacktrace

#0  0x00000000005239f0 in __asan::ReportGenericError(unsigned long, unsigned long, unsigned long, unsigned long, bool, unsigned long, unsigned int, bool) ()
#1  0x0000000000524338 in __asan_report_load1 ()
#2  0x0000000000581842 in FixedKeyHashing (aeskey=0x60f000007750, outbuf=0x7ffff38a27f0 "\254\066\033\357\234>", <incomplete sequence \357>, inbuf=0x7ffff36ff7f0 "\254\066\033\357\234><\357df", tmpbuf=0x603000017f50 "S\t\033\357\234><\357df", id=16383, bytessecparam=24, crypt=0x61700000fc80) at ../../abycore/ot/ot-ext.h:249
#3  0x000000000058804e in OTExtSnd::HashValues (this=0x610000007f40, Q=0x7fffde24b810, seedbuf=0x60b00000af98, snd_buf=0x60b00000aee8, U=0x60400000bf90, OT_ptr=0, OT_len=16384, mat_mul=0x0) at ../../abycore/ot/ot-ext-snd.cpp:216
#4  0x000000000056fa0a in IKNPOTExtSnd::sender_routine (this=0x610000007f40, id=0, myNumOTs=156896) at ../../abycore/ot/iknp-ot-ext-snd.cpp:104
#5  0x000000000058aaf2 in OTExtSnd::OTSenderThread::ThreadMain (this=0x606000011f60) at ../../abycore/ot/ot-ext-snd.h:75
#6  0x00000000005823f6 in CThread::ThreadMainHandler (p=0x606000011f60) at ../../abycore/ot/../util/thread.h:149
#7  0x00007ffff772a454 in start_thread () from /usr/lib/libpthread.so.0
#8  0x00007ffff60d47df in clone () from /usr/lib/libc.so.6

In IKNPOTExtSnd::sender_routine the CBitVector Q is created with wd_size_bits * OTsPerIteration = 128 * 16384 bit and passed to OTExtSnd::HashValues here.

OTExtSnd::HashValues is called with OT_len=16384. It access the buffer of Q via the uint64_t* Qptr. In each iteration of the main loop Qptr is incremented by two (128 bit). Qptr is then passed to FixedKeyHashing together with the parameter hashinbytelen. The function uses this parameter as bytewise index to Qptr (they are called bytessecparam and inbuf inside the function). When we look at the definition of hashinbytelen

uint32_t rowbytelen = bits_in_bytes(m_nBaseOTs);  // == 16
uint32_t hashinbytelen = rowbytelen + sizeof(uint64_t);  // == 16 + 8 == 24

it is clear that FixedKeyHashing access 24 instead of 16 bytes of Q in each iteration. Results:

• Buffer overflow in the last iteration
• The "extra" 8 byte written by FixedKeyHashing are overwritten in the next iteration

Complete ASan output:

==9363==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f330fb4a800 at pc 0x000000581842 bp 0x7f3310b4b230 sp 0x7f3310b4b228
READ of size 1 at 0x7f330fb4a800 thread T10
    #0 0x581841 in FixedKeyHashing(evp_cipher_ctx_st*, unsigned char*, unsigned char*, unsigned char*, unsigned long, unsigned int, crypto*) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/ot-ext.h:249:27
    #1 0x58804d in OTExtSnd::HashValues(CBitVector*, CBitVector*, CBitVector*, CBitVector*, unsigned long, unsigned long, unsigned long**) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/ot-ext-snd.cpp:216:5
    #2 0x56fa09 in IKNPOTExtSnd::sender_routine(unsigned int, unsigned long) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/iknp-ot-ext-snd.cpp:104:3
    #3 0x58aaf1 in OTExtSnd::OTSenderThread::ThreadMain() /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/ot-ext-snd.h:75:24
    #4 0x5823f5 in CThread::ThreadMainHandler(void*) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/../util/thread.h:149:10
    #5 0x7f332a03e453 in start_thread (/usr/lib/libpthread.so.0+0x7453)
    #6 0x7f33289e87de in __GI___clone (/usr/lib/libc.so.6+0xe87de)

0x7f330fb4a800 is located 0 bytes to the right of 262144-byte region [0x7f330fb0a800,0x7f330fb4a800)
allocated by thread T10 here:
    #0 0x519690 in calloc (/home/lennart/git/ABY-upstream/bin/sha1.exe+0x519690)
    #1 0x73b18d in CBitVector::Create(unsigned long) /home/lennart/git/ABY-upstream/src/examples/sha1/../../abycore/util/cbitvector.cpp:51:20
    #2 0x5685b7 in CBitVector::CBitVector(unsigned int) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/../util/cbitvector.h:138:3
    #3 0x56f0ff in IKNPOTExtSnd::sender_routine(unsigned int, unsigned long) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/iknp-ot-ext-snd.cpp:46:13
    #4 0x58aaf1 in OTExtSnd::OTSenderThread::ThreadMain() /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/ot-ext-snd.h:75:24
    #5 0x5823f5 in CThread::ThreadMainHandler(void*) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/../util/thread.h:149:10
    #6 0x7f332a03e453 in start_thread (/usr/lib/libpthread.so.0+0x7453)

Thread T10 created by T1 here:
    #0 0x47c00d in pthread_create (/home/lennart/git/ABY-upstream/bin/sha1.exe+0x47c00d)
    #1 0x581306 in CThread::Start() /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/../util/thread.h:124:17
    #2 0x586bad in OTExtSnd::start_send(unsigned int) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/ot-ext-snd.cpp:45:16
    #3 0x586260 in OTExtSnd::send(unsigned long, unsigned long, unsigned long, CBitVector**, snd_ot_flavor, rec_ot_flavor, unsigned int, MaskingFunction*) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/ot-ext-snd.cpp:24:9
    #4 0x703ec1 in ABYSetup::ThreadRunIKNPSnd(unsigned int) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/aby/abysetup.cpp:206:30
    #5 0x706d5f in ABYSetup::CWorkerThread::ThreadMain() /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/aby/abysetup.cpp:469:29
    #6 0x5823f5 in CThread::ThreadMainHandler(void*) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/../util/thread.h:149:10
    #7 0x7f332a03e453 in start_thread (/usr/lib/libpthread.so.0+0x7453)

Thread T1 created by T0 here:
    #0 0x47c00d in pthread_create (/home/lennart/git/ABY-upstream/bin/sha1.exe+0x47c00d)
    #1 0x581306 in CThread::Start() /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/../util/thread.h:124:17
    #2 0x701b86 in ABYSetup::Init() /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/aby/abysetup.cpp:48:18
    #3 0x7015c1 in ABYSetup::ABYSetup(crypto*, unsigned int, e_role, e_mt_gen_alg) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/aby/abysetup.cpp:27:7
    #4 0x6d754b in ABYParty::Init() /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/aby/abyparty.cpp:103:17
    #5 0x6d6970 in ABYParty::ABYParty(e_role, char*, SECURITYLEVELS, unsigned int, unsigned int, e_mt_gen_alg, unsigned int, unsigned short) /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/aby/abyparty.cpp:54:2
    #6 0x554725 in test_sha1_circuit(e_role, char*, SECURITYLEVELS, unsigned int, unsigned int, e_mt_gen_alg, e_sharing) /home/lennart/git/ABY-upstream/src/examples/sha1/common/sha1_circuit.cpp:24:24
    #7 0x561db3 in main (/home/lennart/git/ABY-upstream/bin/sha1.exe+0x561db3)
    #8 0x7f3328920290 in __libc_start_main (/usr/lib/libc.so.6+0x20290)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/lennart/git/ABY-upstream/src/examples/lowmc/../../abycore/ot/ot-ext.h:249:27 in FixedKeyHashing(evp_cipher_ctx_st*, unsigned char*, unsigned char*, unsigned char*, unsigned long, unsigned int, crypto*)
Shadow bytes around the buggy address:
  0x0fe6e1f614b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fe6e1f614c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fe6e1f614d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fe6e1f614e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0fe6e1f614f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0fe6e1f61500:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0fe6e1f61510: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0fe6e1f61520: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0fe6e1f61530: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0fe6e1f61540: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0fe6e1f61550: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==9363==ABORTING

[MichaelZohner]

Thanks for reporting. The wrong parameter was passed into the method, the updated version uses the correct parameter.