Fix a possible UAF

[zouyonghao]

Got a UAF on an old version of Dqlite. This should fix it.

==2760859==ERROR: AddressSanitizer: heap-use-after-free on address 0x613000020140 at pc 0x7f3514979bbc bp 0x7f34c73cc720 sp 0x7f34c73cc710
READ of size 1 at 0x613000020140 thread T8
    #0 0x7f3514979bbb  (/usr/local/lib/libraft.so.2+0x67bbb)     /home/zyh/raft/src/uv_segment.c:91
    #1 0x7f351466d780  (/lib/x86_64-linux-gnu/libc.so.6+0x45780)
    #2 0x7f351466d6a1  (/lib/x86_64-linux-gnu/libc.so.6+0x456a1)
    #3 0x7f351466d684  (/lib/x86_64-linux-gnu/libc.so.6+0x45684)
    #4 0x7f351466dc9d  (/lib/x86_64-linux-gnu/libc.so.6+0x45c9d)
    #5 0x7f3514972bb4  (/usr/local/lib/libraft.so.2+0x60bb4)              /home/zyh/raft/src/uv_list.c:112
    #6 0x7f351498a283  (/usr/local/lib/libraft.so.2+0x78283)              /home/zyh/raft/src/uv_snapshot.c:715
    #7 0x7f351446651d  (/lib/x86_64-linux-gnu/libuv.so.1+0xc51d)
    #8 0x7f3514a01608  (/lib/x86_64-linux-gnu/libpthread.so.0+0x8608)
    #9 0x7f3514747352  (/lib/x86_64-linux-gnu/libc.so.6+0x11f352)

0x613000020140 is located 64 bytes inside of 384-byte region [0x613000020100,0x613000020280)
freed by thread T8 here:
    #0 0x7f3514b4540f  (/lib/x86_64-linux-gnu/libasan.so.5+0x10d40f)
    #1 0x7f3514972b18  (/usr/local/lib/libraft.so.2+0x60b18)              /home/zyh/raft/src/uv_list.c:104
    #2 0x7f351498a283  (/usr/local/lib/libraft.so.2+0x78283)              /home/zyh/raft/src/uv_snapshot.c:715
    #3 0x7f351446651d  (/lib/x86_64-linux-gnu/libuv.so.1+0xc51d)

previously allocated by thread T8 here:
    #0 0x7f3514b45c3e  (/lib/x86_64-linux-gnu/libasan.so.5+0x10dc3e)
    #1 0x7f351497c366  (/usr/local/lib/libraft.so.2+0x6a366)              /home/zyh/raft/src/uv_segment.c:74
    #2 0x7f3514972e3b  (/usr/local/lib/libraft.so.2+0x60e3b)              /home/zyh/raft/src/uv_list.c:88
    #3 0x7f351498a283  (/usr/local/lib/libraft.so.2+0x78283)              /home/zyh/raft/src/uv_snapshot.c:715
    #4 0x7f351446651d  (/lib/x86_64-linux-gnu/libuv.so.1+0xc51d)

Thread T8 created by T7 here:
    #0 0x7f3514a72815  (/lib/x86_64-linux-gnu/libasan.so.5+0x3a815)
    #1 0x7f35144771e7  (/lib/x86_64-linux-gnu/libuv.so.1+0x1d1e7)

Thread T7 created by T0 here:
    #0 0x7f3514a72815  (/lib/x86_64-linux-gnu/libasan.so.5+0x3a815)
    #1 0x7f351487a4ff  (/usr/local/lib/libdqlite.so.0+0x604ff)

SUMMARY: AddressSanitizer: heap-use-after-free (/usr/local/lib/libraft.so.2+0x67bbb) 
Shadow bytes around the buggy address:
  0x0c267fffbfd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c267fffbfe0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c267fffbff0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c267fffc000: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c267fffc010: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
=>0x0c267fffc020: fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd fd fd
  0x0c267fffc030: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c267fffc040: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c267fffc050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c267fffc060: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c267fffc070: 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
  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
==2760859==ABORTING

[cole-miller]

Thanks for reporting this and submitting a fix! I have a couple of questions:

• Are you able to reproduce this UAF or was it a one-off?
• What release/commit of libraft are you using?

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 77.76%. Comparing base (ee07596) to head (f98027e).
Report is 9 commits behind head on master.

Files	Patch %	Lines
src/raft/uv_list.c	0.00%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #669      +/-   ##
==========================================
+ Coverage   77.56%   77.76%   +0.19%     
==========================================
  Files         197      197              
  Lines       27638    27654      +16     
  Branches     5486     5487       +1     
==========================================
+ Hits        21438    21505      +67     
- Misses       4326     4327       +1     
+ Partials     1874     1822      -52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zouyonghao]

Thanks for reporting this and submitting a fix! I have a couple of questions:

• Are you able to reproduce this UAF or was it a one-off?
• What release/commit of libraft are you using?

Thanks for your reply.

• It happens twice on my side. Currently, I didn't try to reproduce it.
• The version is v1.15.0

[letFunny]

Just a question, thanks for the PR!

[letFunny]

This is not related to the PR strictly but what does rv != 0 mean here @cole-miller. It is set in the above loop and it is overwritten every iteration so rv here is the return code of the last entry that we process, right? We are ignoring the return codes of all the entries but this one, is that sound?

[cole-miller]

The key is lines 64-72, if we get one nonzero return code from UvSnapshotInfoAppendIfMatch or uvSegmentInfoAppendIfMatch then we skip processing the rest of the files in the list. You're right, I wasn't reading closely enough, those lines don't work as intended because of uv_fs_scandir_next just above.