varlink / libvarlink

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

Fix memory leaks #52

Closed t-8ch closed 1 year ago

t-8ch commented 2 years ago

Fixes #51

evverx commented 1 year ago

@t-8ch thanks! I totally forgot about that memory leak. I also found two more issues at the time but decided not to report them because I wasn't sure whether the project was alive. Looks like it is so I'll leave them here just in case:

heap-use-after-free in test-server-client ```sh CC=clang meson -Db_sanitize=address -Db_lundef=false build meson test -C ./build/ --print-errorlog test-server-client ``` ```sh ninja: Entering directory `/libvarlink/build' ninja: no work to do. 1/1 test-server-client FAIL 0.30s exit status 1 >>> MALLOC_PERTURB_=82 /libvarlink/build/lib/test-server-client ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀ ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― stderr: ================================================================= ==2638==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000000b10 at pc 0x00000052a1ed bp 0x7ffff15b0230 sp 0x7ffff15b0228 READ of size 8 at 0x606000000b10 thread T0 #0 0x52a1ec in varlink_call_reply /libvarlink/build/../lib/service.c:660:23 #1 0x518172 in org_varlink_example_Echo /libvarlink/build/../lib/test-server-client.c:33:9 #2 0x526bb2 in varlink_service_method_callback /libvarlink/build/../lib/service.c:282:16 #3 0x5298f2 in varlink_service_dispatch_connection /libvarlink/build/../lib/service.c:555:29 #4 0x528bee in varlink_service_process_events /libvarlink/build/../lib/service.c:604:29 #5 0x518a8e in test_process_events /libvarlink/build/../lib/test-server-client.c:66:25 #6 0x5173e5 in main /libvarlink/build/../lib/test-server-client.c:155:25 #7 0x7fb5e6f4750f in __libc_start_call_main (/lib64/libc.so.6+0x2750f) (BuildId: 765237b0355c030ff41d969eedcb87bfccb43595) #8 0x7fb5e6f475c8 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x275c8) (BuildId: 765237b0355c030ff41d969eedcb87bfccb43595) #9 0x41e414 in _start (/libvarlink/build/lib/test-server-client+0x41e414) (BuildId: 895f2f71b3295e0d2de926f12b3f985f4b8f704d) 0x606000000b10 is located 16 bytes inside of 64-byte region [0x606000000b00,0x606000000b40) freed by thread T0 here: #0 0x4d2138 in __interceptor_free.part.0 asan_malloc_linux.cpp.o #1 0x52523c in varlink_call_unref /libvarlink/build/../lib/service.c:101:17 #2 0x52a1bf in varlink_call_reply /libvarlink/build/../lib/service.c:660:42 #3 0x518172 in org_varlink_example_Echo /libvarlink/build/../lib/test-server-client.c:33:9 #4 0x526bb2 in varlink_service_method_callback /libvarlink/build/../lib/service.c:282:16 #5 0x5298f2 in varlink_service_dispatch_connection /libvarlink/build/../lib/service.c:555:29 #6 0x528bee in varlink_service_process_events /libvarlink/build/../lib/service.c:604:29 #7 0x518a8e in test_process_events /libvarlink/build/../lib/test-server-client.c:66:25 #8 0x5173e5 in main /libvarlink/build/../lib/test-server-client.c:155:25 #9 0x7fb5e6f4750f in __libc_start_call_main (/lib64/libc.so.6+0x2750f) (BuildId: 765237b0355c030ff41d969eedcb87bfccb43595) previously allocated by thread T0 here: #0 0x4d3457 in __interceptor_calloc (/libvarlink/build/lib/test-server-client+0x4d3457) (BuildId: 895f2f71b3295e0d2de926f12b3f985f4b8f704d) #1 0x52b8df in varlink_call_new /libvarlink/build/../lib/service.c:69:16 #2 0x52970e in varlink_service_dispatch_connection /libvarlink/build/../lib/service.c:551:29 #3 0x528bee in varlink_service_process_events /libvarlink/build/../lib/service.c:604:29 #4 0x518a8e in test_process_events /libvarlink/build/../lib/test-server-client.c:66:25 #5 0x5173e5 in main /libvarlink/build/../lib/test-server-client.c:155:25 #6 0x7fb5e6f4750f in __libc_start_call_main (/lib64/libc.so.6+0x2750f) (BuildId: 765237b0355c030ff41d969eedcb87bfccb43595) SUMMARY: AddressSanitizer: heap-use-after-free /libvarlink/build/../lib/service.c:660:23 in varlink_call_reply Shadow bytes around the buggy address: 0x0c0c7fff8110: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00 0x0c0c7fff8120: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa 0x0c0c7fff8130: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00 0x0c0c7fff8140: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00 0x0c0c7fff8150: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa =>0x0c0c7fff8160: fd fd[fd]fd fd fd fd fd fa fa fa fa fa fa fa fa 0x0c0c7fff8170: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c0c7fff8180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c0c7fff8190: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c0c7fff81a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c0c7fff81b0: 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 ==2638==ABORTING ```
Conditional jump or move depends on uninitialised value(s) in varlink_array_unrefp ```sh meson build ninja -C ./build printf '{"":[\0' | valgrind --track-origins=yes ./build/tool/varlink bridge ``` ```sh ==2931== Memcheck, a memory error detector ==2931== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. ==2931== Using Valgrind-3.20.0 and LibVEX; rerun with -h for copyright info ==2931== Command: ./build/tool/varlink bridge ==2931== ==2931== Conditional jump or move depends on uninitialised value(s) ==2931== at 0x410C7A: varlink_value_clear (value.c:13) ==2931== by 0x4073CA: varlink_array_unref (array.c:110) ==2931== by 0x407427: varlink_array_unrefp (array.c:121) ==2931== by 0x407352: varlink_array_new_from_scanner (array.c:54) ==2931== by 0x410DAE: varlink_value_read_from_scanner (value.c:56) ==2931== by 0x40B0FD: varlink_object_new_from_scanner (object.c:122) ==2931== by 0x40B247: varlink_object_new_from_json (object.c:156) ==2931== by 0x40DDB9: varlink_stream_read (stream.c:229) ==2931== by 0x40448C: handleBridge (command-bridge.c:147) ==2931== by 0x404C63: bridge_run (command-bridge.c:339) ==2931== by 0x40353B: cli_run (cli.c:496) ==2931== by 0x40700D: main (main.c:14) ==2931== Uninitialised value was created by a heap allocation ==2931== at 0x484378A: malloc (vg_replace_malloc.c:392) ==2931== by 0x484870B: realloc (vg_replace_malloc.c:1451) ==2931== by 0x4070DF: array_append (array.c:22) ==2931== by 0x407276: varlink_array_new_from_scanner (array.c:73) ==2931== by 0x410DAE: varlink_value_read_from_scanner (value.c:56) ==2931== by 0x40B0FD: varlink_object_new_from_scanner (object.c:122) ==2931== by 0x40B247: varlink_object_new_from_json (object.c:156) ==2931== by 0x40DDB9: varlink_stream_read (stream.c:229) ==2931== by 0x40448C: handleBridge (command-bridge.c:147) ==2931== by 0x404C63: bridge_run (command-bridge.c:339) ==2931== by 0x40353B: cli_run (cli.c:496) ==2931== by 0x40700D: main (main.c:14) ==2931== ==2931== Conditional jump or move depends on uninitialised value(s) ==2931== at 0x410C7F: varlink_value_clear (value.c:13) ==2931== by 0x4073CA: varlink_array_unref (array.c:110) ==2931== by 0x407427: varlink_array_unrefp (array.c:121) ==2931== by 0x407352: varlink_array_new_from_scanner (array.c:54) ==2931== by 0x410DAE: varlink_value_read_from_scanner (value.c:56) ==2931== by 0x40B0FD: varlink_object_new_from_scanner (object.c:122) ==2931== by 0x40B247: varlink_object_new_from_json (object.c:156) ==2931== by 0x40DDB9: varlink_stream_read (stream.c:229) ==2931== by 0x40448C: handleBridge (command-bridge.c:147) ==2931== by 0x404C63: bridge_run (command-bridge.c:339) ==2931== by 0x40353B: cli_run (cli.c:496) ==2931== by 0x40700D: main (main.c:14) ==2931== Uninitialised value was created by a heap allocation ==2931== at 0x484378A: malloc (vg_replace_malloc.c:392) ==2931== by 0x484870B: realloc (vg_replace_malloc.c:1451) ==2931== by 0x4070DF: array_append (array.c:22) ==2931== by 0x407276: varlink_array_new_from_scanner (array.c:73) ==2931== by 0x410DAE: varlink_value_read_from_scanner (value.c:56) ==2931== by 0x40B0FD: varlink_object_new_from_scanner (object.c:122) ==2931== by 0x40B247: varlink_object_new_from_json (object.c:156) ==2931== by 0x40DDB9: varlink_stream_read (stream.c:229) ==2931== by 0x40448C: handleBridge (command-bridge.c:147) ==2931== by 0x404C63: bridge_run (command-bridge.c:339) ==2931== by 0x40353B: cli_run (cli.c:496) ==2931== by 0x40700D: main (main.c:14) ==2931== ==2931== Conditional jump or move depends on uninitialised value(s) ==2931== at 0x410C84: varlink_value_clear (value.c:13) ==2931== by 0x4073CA: varlink_array_unref (array.c:110) ==2931== by 0x407427: varlink_array_unrefp (array.c:121) ==2931== by 0x407352: varlink_array_new_from_scanner (array.c:54) ==2931== by 0x410DAE: varlink_value_read_from_scanner (value.c:56) ==2931== by 0x40B0FD: varlink_object_new_from_scanner (object.c:122) ==2931== by 0x40B247: varlink_object_new_from_json (object.c:156) ==2931== by 0x40DDB9: varlink_stream_read (stream.c:229) ==2931== by 0x40448C: handleBridge (command-bridge.c:147) ==2931== by 0x404C63: bridge_run (command-bridge.c:339) ==2931== by 0x40353B: cli_run (cli.c:496) ==2931== by 0x40700D: main (main.c:14) ==2931== Uninitialised value was created by a heap allocation ==2931== at 0x484378A: malloc (vg_replace_malloc.c:392) ==2931== by 0x484870B: realloc (vg_replace_malloc.c:1451) ==2931== by 0x4070DF: array_append (array.c:22) ==2931== by 0x407276: varlink_array_new_from_scanner (array.c:73) ==2931== by 0x410DAE: varlink_value_read_from_scanner (value.c:56) ==2931== by 0x40B0FD: varlink_object_new_from_scanner (object.c:122) ==2931== by 0x40B247: varlink_object_new_from_json (object.c:156) ==2931== by 0x40DDB9: varlink_stream_read (stream.c:229) ==2931== by 0x40448C: handleBridge (command-bridge.c:147) ==2931== by 0x404C63: bridge_run (command-bridge.c:339) ==2931== by 0x40353B: cli_run (cli.c:496) ==2931== by 0x40700D: main (main.c:14) ==2931== ==2931== Conditional jump or move depends on uninitialised value(s) ==2931== at 0x410C89: varlink_value_clear (value.c:13) ==2931== by 0x4073CA: varlink_array_unref (array.c:110) ==2931== by 0x407427: varlink_array_unrefp (array.c:121) ==2931== by 0x407352: varlink_array_new_from_scanner (array.c:54) ==2931== by 0x410DAE: varlink_value_read_from_scanner (value.c:56) ==2931== by 0x40B0FD: varlink_object_new_from_scanner (object.c:122) ==2931== by 0x40B247: varlink_object_new_from_json (object.c:156) ==2931== by 0x40DDB9: varlink_stream_read (stream.c:229) ==2931== by 0x40448C: handleBridge (command-bridge.c:147) ==2931== by 0x404C63: bridge_run (command-bridge.c:339) ==2931== by 0x40353B: cli_run (cli.c:496) ==2931== by 0x40700D: main (main.c:14) ==2931== Uninitialised value was created by a heap allocation ==2931== at 0x484378A: malloc (vg_replace_malloc.c:392) ==2931== by 0x484870B: realloc (vg_replace_malloc.c:1451) ==2931== by 0x4070DF: array_append (array.c:22) ==2931== by 0x407276: varlink_array_new_from_scanner (array.c:73) ==2931== by 0x410DAE: varlink_value_read_from_scanner (value.c:56) ==2931== by 0x40B0FD: varlink_object_new_from_scanner (object.c:122) ==2931== by 0x40B247: varlink_object_new_from_json (object.c:156) ==2931== by 0x40DDB9: varlink_stream_read (stream.c:229) ==2931== by 0x40448C: handleBridge (command-bridge.c:147) ==2931== by 0x404C63: bridge_run (command-bridge.c:339) ==2931== by 0x40353B: cli_run (cli.c:496) ==2931== by 0x40700D: main (main.c:14) ==2931== ==2931== Conditional jump or move depends on uninitialised value(s) ==2931== at 0x410C8E: varlink_value_clear (value.c:13) ==2931== by 0x4073CA: varlink_array_unref (array.c:110) ==2931== by 0x407427: varlink_array_unrefp (array.c:121) ==2931== by 0x407352: varlink_array_new_from_scanner (array.c:54) ==2931== by 0x410DAE: varlink_value_read_from_scanner (value.c:56) ==2931== by 0x40B0FD: varlink_object_new_from_scanner (object.c:122) ==2931== by 0x40B247: varlink_object_new_from_json (object.c:156) ==2931== by 0x40DDB9: varlink_stream_read (stream.c:229) ==2931== by 0x40448C: handleBridge (command-bridge.c:147) ==2931== by 0x404C63: bridge_run (command-bridge.c:339) ==2931== by 0x40353B: cli_run (cli.c:496) ==2931== by 0x40700D: main (main.c:14) ==2931== Uninitialised value was created by a heap allocation ==2931== at 0x484378A: malloc (vg_replace_malloc.c:392) ==2931== by 0x484870B: realloc (vg_replace_malloc.c:1451) ==2931== by 0x4070DF: array_append (array.c:22) ==2931== by 0x407276: varlink_array_new_from_scanner (array.c:73) ==2931== by 0x410DAE: varlink_value_read_from_scanner (value.c:56) ==2931== by 0x40B0FD: varlink_object_new_from_scanner (object.c:122) ==2931== by 0x40B247: varlink_object_new_from_json (object.c:156) ==2931== by 0x40DDB9: varlink_stream_read (stream.c:229) ==2931== by 0x40448C: handleBridge (command-bridge.c:147) ==2931== by 0x404C63: bridge_run (command-bridge.c:339) ==2931== by 0x40353B: cli_run (cli.c:496) ==2931== by 0x40700D: main (main.c:14) ==2931== ==2931== ```

I moved those bugs to https://github.com/varlink/libvarlink/issues/55 and https://github.com/varlink/libvarlink/issues/54