varlink / libvarlink

C implementation of the Varlink protocol and command line tool
Apache License 2.0
87 stars 15 forks source link

Fix memory errors #56

Closed t-8ch closed 1 year ago

t-8ch commented 1 year ago

as reported by @evverx in https://github.com/varlink/libvarlink/pull/52#issuecomment-1328470686

evverx commented 1 year ago

Looks like the issues are gone. Thanks! I'll go ahead and close #54 and #55.

@t-8ch FWIW if libvarlink is more or less widely used it should be possible to start fuzzing it on OSS-Fuzz. I think I even wrote a fuzz target some time ago (and that's where the memory leak and the unitialized value came from as far as I can remember). I can't seem to find a lot of dependencies in various distributions but if it's used in the wild somewhere it should do in terms of convincing OSS-Fuzz to accept it.

evverx commented 1 year ago

On second thought the library doesn't seem to change very often so it's probably easier to just ressurect the fuzz target, take the varlink corpus I use to fuzz systemd and run it for a day or two.

haraldh commented 1 year ago

Thank you!

evverx commented 1 year ago

I've just resurrected the fuzz target along with a patch I needed to silence varlink_object_new_from_json: https://github.com/varlink/libvarlink/pull/57.

evverx commented 1 year ago

So far it has found only

==2330383==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x547247 in json_write_string /src/libvarlink/build/../lib/value.c:98:9
    #1 0x547247 in varlink_value_write_json /src/libvarlink/build/../lib/value.c:190:29
    #2 0x53bdb0 in varlink_object_write_json /src/libvarlink/build/../lib/object.c:493:21
    #3 0x53cbab in varlink_object_to_pretty_json /src/libvarlink/build/../lib/object.c:536:13
    #4 0x53cbab in varlink_object_to_json /src/libvarlink/build/../lib/object.c:573:15
    #5 0x5347f5 in LLVMFuzzerTestOneInput /src/fuzz-object.c:38:15
    #6 0x43dbc3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
    #7 0x429322 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
    #8 0x42ebcc in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
    #9 0x458102 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #10 0x7f905cc2950f in __libc_start_call_main (/lib64/libc.so.6+0x2950f) (BuildId: 85c438f4ff93e21675ff174371c9c583dca00b2c)
    #11 0x7f905cc295c8 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x295c8) (BuildId: 85c438f4ff93e21675ff174371c9c583dca00b2c)
    #12 0x41f4ed in _start (/home/vagrant/oss-fuzz-2/build/out/libvarlink/fuzz-object+0x41f4ed)

  Uninitialized value was created by a heap allocation
    #0 0x4e1f2c in __interceptor_realloc /src/llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:883:3
    #1 0x7f905cc81837 in _IO_mem_finish (/lib64/libc.so.6+0x81837) (BuildId: 85c438f4ff93e21675ff174371c9c583dca00b2c)
    #2 0x704fffffffff  (<unknown module>)

but looking at the origin of the uninitialized value I'm inclined to say that it's probably a MSan false positive. valgrind can't confirm it either. I'll be back in a day or two if the fuzz target finds anything else.