arangodb / velocypack

A fast and compact format for serialization and storage
Other
420 stars 40 forks source link

Crash when parsing malformed VPACK file #109

Open retpoline opened 2 years ago

retpoline commented 2 years ago

Hi folks,

A crash was found while fuzz testing of the vpack-to-json binary which can be triggered via a malformed VPACK file. Although this malformed file only crashes the program as-is, it could potentially be crafted further and create a security issue where these kinds of files would be able compromise the process's memory through taking advantage of affordances given by memory corruption. It's recommend to harden the code to prevent these kinds of bugs as it could greatly mitigate such this issue and even future bugs.

Link to crash.vpack (size ~1kb): https://ufile.io/80isjc53

debug log

$ gdb -q vpack-to-json
Reading symbols from vpack-to-json...
(No debugging symbols found in vpack-to-json)

(gdb) r crash.vpack test.json
Starting program: vpack-to-json crash.vpack test.json

Program received signal SIGSEGV, Segmentation fault.
0x00005555555658cf in arangodb::velocypack::Validator::validateIndexedArray(unsigned char const*, unsigned long) ()
(gdb) bt
#0  0x00005555555658cf in arangodb::velocypack::Validator::validateIndexedArray(unsigned char const*, unsigned long) ()
#1  0x0000555555563cb1 in arangodb::velocypack::Validator::validate(unsigned char const*, unsigned long, bool) ()
#2  0x0000555555563fd6 in arangodb::velocypack::Validator::validateCompactArray(unsigned char const*, unsigned long) ()
#3  0x0000555555563cb1 in arangodb::velocypack::Validator::validate(unsigned char const*, unsigned long, bool) ()
#4  0x0000555555565b37 in arangodb::velocypack::Validator::validateCompactObject(unsigned char const*, unsigned long) ()
#5  0x0000555555563ccc in arangodb::velocypack::Validator::validate(unsigned char const*, unsigned long, bool) ()
#6  0x0000555555558a06 in main ()

(gdb) i r
rax            0x5555555658cf      93824992303311
rbx            0x9                 9
rcx            0x9095e5e5e622c7d   651155380535176317
rdx            0x555555576d8c      93824992374156
rsi            0x12                18
rdi            0x7fffffff5d50      140737488313680
rbp            0xffffffffffffff9f  0xffffffffffffff9f
rsp            0x7fffffff5ab0      0x7fffffff5ab0
r8             0x9f                159
r9             0x49                73
r10            0x1                 1
r11            0x9f                159
r12            0x7fffffff5d50      140737488313680
r13            0x909090909090901   651061555542690049
r14            0x9                 9
r15            0x8                 8
rip            0x5555555658cf      0x5555555658cf <arangodb::velocypack::Validator::validateIndexedArray(unsigned char const*, unsigned long)+2463>
eflags         0x10207             [ CF PF IF RF ]
cs             0x33                51
ss             0x2b                43
ds             0x0                 0
es             0x0                 0
fs             0x0                 0
gs             0x0                 0

(gdb) x/i $rip
=> 0x5555555658cf <_ZN8arangodb10velocypack9Validator20validateIndexedArrayEPKhm+2463>: mov    (%rcx),%rax

(gdb) exploitable
Description: Access violation
Short description: AccessViolation (21/22)
Hash: 603fccd8b3402844f8eba2f2f993f65e.ce3b08e78bc48a581afa91728bdd6f45
Exploitability Classification: UNKNOWN
Explanation: The target crashed due to an access violation but there is not enough additional information available to determine exploitability.
jsteemann commented 2 years ago

@retpoline : Thanks for the bug report. I fully agree this should be fixed. Will look into it.

jsteemann commented 2 years ago

@retpoline : I have created a PR with a potential fix for the problem: https://github.com/arangodb/velocypack/pull/110 Do you mind trying if the changes in the PR fix the problem you reported?

In addition, is it possible to share your fuzzer code with us? I would like to use it if possible and ideally also integrate it into our testing. Not sure if this is possible, but it would be nice if you could tell me a bit more about the fuzzer. Thanks!

retpoline commented 2 years ago

Looks good!

caught exception: Array items number is out of bounds

Sure, the fuzzer used was litefuzz -- it's very straightward to use if you'd like to run it again or try and fuzz out bugs on other binaries.

jsteemann commented 2 years ago

@retpoline : we'll try to integrate the fuzzer into our testing. Thanks for bringing up the issue!

jsteemann commented 2 years ago

@retpoline : for info, we tried running litefuzz ourselves, and it produced a few additional issues. These should be fixed via https://github.com/arangodb/velocypack/pull/111.

retpoline commented 2 years ago

@jsteemann glad I could get the conversation started! nice work.