| Message ID | 20250407215308.9674-1-absicsz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9e9c1cb5833407b9c1cb2b7f7900b27194787551 |
| Headers | show |
| Series | smsutil: fix possible buffer overflow | expand |
Hello: This patch was applied to ofono.git (master) by Denis Kenzior <denkenz@gmail.com>: On Mon, 7 Apr 2025 23:52:50 +0200 you wrote: > Adding the null terminator is not necessary since encode_hex_own_address() > already provides it. The bug was discovered via ASAN: > > ==2244==ERROR: AddressSanitizer: stack-buffer-overflow on address 0xffffa141b839 at pc 0x0000008d2ac0 bp 0xfffffea95f00 sp 0xfffffea95f18 > WRITE of size 1 at 0xffffa141b839 thread T0 > #0 0x8d2abc in sms_address_to_hex_string src/smsutil.c:2418 > #1 0x8d3ac0 in sms_assembly_store src/smsutil.c:2509 > #2 0x8d5fdc in sms_assembly_add_fragment_backup src/smsutil.c:2696 > #3 0x8d4bb8 in sms_assembly_add_fragment src/smsutil.c:2603 > #4 0x88c10c in handle_deliver src/sms.c:1442 > #5 0x88cff4 in ofono_sms_deliver_notify src/sms.c:1638 > #6 0x58b7ac in raw_read_cb drivers/qmimodem/sms.c:403 > #7 0x55e6cc in service_send_callback drivers/qmimodem/qmi.c:2476 > #8 0x549fc4 in __rx_message drivers/qmimodem/qmi.c:801 > #9 0x54cfdc in received_qmux_data drivers/qmimodem/qmi.c:1043 > #10 0xaad880 in io_callback ell/io.c:105 > #11 0xaa7e1c in l_main_iterate ell/main.c:461 > #12 0x807958 in event_check src/main.c:182 > #13 0xffffa3fdf964 (/lib/aarch64-linux-gnu/libglib-2.0.so.0+0x5f964) (BuildId: 3901bdcbc847d04fc971a1923bed26ef7d9b81e4) > #14 0xffffa3fe03b4 (/lib/aarch64-linux-gnu/libglib-2.0.so.0+0x603b4) (BuildId: 3901bdcbc847d04fc971a1923bed26ef7d9b81e4) > #15 0xffffa3fe10e0 in g_main_loop_run (/lib/aarch64-linux-gnu/libglib-2.0.so.0+0x610e0) (BuildId: 3901bdcbc847d04fc971a1923bed26ef7d9b81e4) > #16 0x808478 in main src/main.c:300 > #17 0xffffa36f2298 (/lib/aarch64-linux-gnu/libc.so.6+0x22298) (BuildId: 8e356c2fd2ec1ebf5535228f366e2af8bd837770) > #18 0xffffa36f2378 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x22378) (BuildId: 8e356c2fd2ec1ebf5535228f366e2af8bd837770) > #19 0x41096c in _start (/home/mobian/ofono/src/ofonod+0x41096c) (BuildId: e672292c782b5f428bf5870e0142347fe81107b2) > > [...] Here is the summary with links: - smsutil: fix possible buffer overflow https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=9e9c1cb58334 You are awesome, thank you!
diff --git a/src/smsutil.c b/src/smsutil.c index 5c9826d3..84895c4f 100644 --- a/src/smsutil.c +++ b/src/smsutil.c @@ -2415,8 +2415,6 @@ gboolean sms_address_to_hex_string(const struct sms_address *in, char *straddr) if (encode_hex_own_buf(pdu, offset, 0, straddr) == NULL) return FALSE; - straddr[offset * 2 + 1] = '\0'; - return TRUE; }
Adding the null terminator is not necessary since encode_hex_own_address() already provides it. The bug was discovered via ASAN: ==2244==ERROR: AddressSanitizer: stack-buffer-overflow on address 0xffffa141b839 at pc 0x0000008d2ac0 bp 0xfffffea95f00 sp 0xfffffea95f18 WRITE of size 1 at 0xffffa141b839 thread T0 #0 0x8d2abc in sms_address_to_hex_string src/smsutil.c:2418 #1 0x8d3ac0 in sms_assembly_store src/smsutil.c:2509 #2 0x8d5fdc in sms_assembly_add_fragment_backup src/smsutil.c:2696 #3 0x8d4bb8 in sms_assembly_add_fragment src/smsutil.c:2603 #4 0x88c10c in handle_deliver src/sms.c:1442 #5 0x88cff4 in ofono_sms_deliver_notify src/sms.c:1638 #6 0x58b7ac in raw_read_cb drivers/qmimodem/sms.c:403 #7 0x55e6cc in service_send_callback drivers/qmimodem/qmi.c:2476 #8 0x549fc4 in __rx_message drivers/qmimodem/qmi.c:801 #9 0x54cfdc in received_qmux_data drivers/qmimodem/qmi.c:1043 #10 0xaad880 in io_callback ell/io.c:105 #11 0xaa7e1c in l_main_iterate ell/main.c:461 #12 0x807958 in event_check src/main.c:182 #13 0xffffa3fdf964 (/lib/aarch64-linux-gnu/libglib-2.0.so.0+0x5f964) (BuildId: 3901bdcbc847d04fc971a1923bed26ef7d9b81e4) #14 0xffffa3fe03b4 (/lib/aarch64-linux-gnu/libglib-2.0.so.0+0x603b4) (BuildId: 3901bdcbc847d04fc971a1923bed26ef7d9b81e4) #15 0xffffa3fe10e0 in g_main_loop_run (/lib/aarch64-linux-gnu/libglib-2.0.so.0+0x610e0) (BuildId: 3901bdcbc847d04fc971a1923bed26ef7d9b81e4) #16 0x808478 in main src/main.c:300 #17 0xffffa36f2298 (/lib/aarch64-linux-gnu/libc.so.6+0x22298) (BuildId: 8e356c2fd2ec1ebf5535228f366e2af8bd837770) #18 0xffffa36f2378 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x22378) (BuildId: 8e356c2fd2ec1ebf5535228f366e2af8bd837770) #19 0x41096c in _start (/home/mobian/ofono/src/ofonod+0x41096c) (BuildId: e672292c782b5f428bf5870e0142347fe81107b2) Address 0xffffa141b839 is located in stack of thread T0 at offset 57 in frame #0 0x8d3970 in sms_assembly_store src/smsutil.c:2501 This frame has 2 object(s): [32, 57) 'straddr' (line 2504) <== Memory access at offset 57 overflows this variable [96, 273) 'buf' (line 2502) HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork (longjmp and C++ exceptions *are* supported) SUMMARY: AddressSanitizer: stack-buffer-overflow src/smsutil.c:2418 in sms_address_to_hex_string Shadow bytes around the buggy address: 0xffffa141b580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0xffffa141b600: f1 f1 f1 f1 04 f2 f8 f8 f2 f2 00 00 00 00 00 00 0xffffa141b680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0xffffa141b700: 00 00 00 00 00 00 00 00 00 00 00 04 f3 f3 f3 f3 0xffffa141b780: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 =>0xffffa141b800: f1 f1 f1 f1 00 00 00[01]f2 f2 f2 f2 00 00 00 00 0xffffa141b880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0xffffa141b900: 00 00 01 f3 f3 f3 f3 f3 f3 f3 f3 f3 00 00 00 00 0xffffa141b980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0xffffa141ba00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0xffffa141ba80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 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 ==2244==ABORTING --- src/smsutil.c | 2 -- 1 file changed, 2 deletions(-)