Eli Zaretskii <
address@hidden> schrieb am Mi., 10. Mai 2017 um 04:43 Uhr:
> From: Philipp Stephani <address@hidden>
> Date: Tue, 09 May 2017 23:15:26 +0000
>
> There's one actual bug: the sockaddr_in in line 3538 of process.c needs to be sockaddr_storage, otherwise
> it's too small for IPv6. (And conv_sockaddr_to_lisp should probably check that the address is of the expected
> length and not blindly overwrite the len argument.)
Please show the detailed analysis, as I looked into that once and
concluded that the code is correct.
The full report is
=================================================================
==31024==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff5fbfa690 at pc 0x0001003e6baf bp 0x7fff5fbfa4f0 sp 0x7fff5fbfa4e8
READ of size 2 at 0x7fff5fbfa690 thread T0
#0 0x1003e6bae in conv_sockaddr_to_lisp src/process.c:2497:34
#1 0x1003eb9f8 in connect_network_socket src/process.c:3542:7
#2 0x1003e9864 in Fmake_network_process src/process.c:4154:5
#3 0x10035e417 in eval_sub src/eval.c:2191:10
#4 0x10035f1a7 in Fsetq src/eval.c:511:10
#5 0x10035dfd1 in eval_sub src/eval.c:2173:8
#6 0x10035efbf in Fprogn src/eval.c:449:13
#7 0x10035dfd1 in eval_sub src/eval.c:2173:8
#8 0x100361df4 in internal_lisp_condition_case src/eval.c:1297:24
#9 0x100361a85 in Fcondition_case src/eval.c:1221:10
#10 0x10035dfd1 in eval_sub src/eval.c:2173:8
#11 0x10035e02f in eval_sub src/eval.c:2208:21
#12 0x10035ef16 in Fand src/eval.c:384:13
#13 0x10035dfd1 in eval_sub src/eval.c:2173:8
#14 0x100360e44 in Fwhile src/eval.c:979:17
#15 0x10035dfd1 in eval_sub src/eval.c:2173:8
#16 0x10035efbf in Fprogn src/eval.c:449:13
#17 0x10035dfd1 in eval_sub src/eval.c:2173:8
#18 0x100361a28 in Funwind_protect src/eval.c:1185:9
#19 0x10035dfd1 in eval_sub src/eval.c:2173:8
#20 0x10035efbf in Fprogn src/eval.c:449:13
#21 0x100360d2c in Flet src/eval.c:963:9
#22 0x10035dfd1 in eval_sub src/eval.c:2173:8
#23 0x10035efbf in Fprogn src/eval.c:449:13
#24 0x10036766f in funcall_lambda src/eval.c:3015:11
#25 0x100364efb in Ffuncall src/eval.c:2746:11
#26 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
#27 0x1003673de in funcall_lambda src/eval.c:2944:11
#28 0x100364efb in Ffuncall src/eval.c:2746:11
#29 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
#30 0x1003673de in funcall_lambda src/eval.c:2944:11
#31 0x100364efb in Ffuncall src/eval.c:2746:11
#32 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
#33 0x1003673de in funcall_lambda src/eval.c:2944:11
#34 0x100364efb in Ffuncall src/eval.c:2746:11
#35 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
#36 0x1003673de in funcall_lambda src/eval.c:2944:11
#37 0x100364efb in Ffuncall src/eval.c:2746:11
#38 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
#39 0x1003673de in funcall_lambda src/eval.c:2944:11
#40 0x100364efb in Ffuncall src/eval.c:2746:11
#41 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
#42 0x1003673de in funcall_lambda src/eval.c:2944:11
#43 0x1003643df in apply_lambda src/eval.c:2881:9
#44 0x10035df97 in eval_sub src/eval.c:2265:12
#45 0x100363d85 in Feval src/eval.c:2042:28
#46 0x100366af0 in funcall_subr src/eval.c:2821:19
#47 0x100364f15 in Ffuncall src/eval.c:2744:11
#48 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
#49 0x1003673de in funcall_lambda src/eval.c:2944:11
#50 0x100364efb in Ffuncall src/eval.c:2746:11
#51 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
#52 0x1003673de in funcall_lambda src/eval.c:2944:11
#53 0x100364efb in Ffuncall src/eval.c:2746:11
#54 0x1003dd245 in exec_byte_code src/bytecode.c:641:12
#55 0x1003673de in funcall_lambda src/eval.c:2944:11
#56 0x1003643df in apply_lambda src/eval.c:2881:9
#57 0x10035df97 in eval_sub src/eval.c:2265:12
#58 0x100363d85 in Feval src/eval.c:2042:28
#59 0x10035e680 in eval_sub src/eval.c:2223:15
#60 0x1003a6944 in readevalloop src/lread.c:1947:15
#61 0x1003a410c in Fload src/lread.c:1359:7
#62 0x10035e917 in eval_sub src/eval.c:2234:15
#63 0x100363d85 in Feval src/eval.c:2042:28
#64 0x10026f0b2 in top_level_2 src/keyboard.c:1121:10
#65 0x1003621f4 in internal_condition_case src/eval.c:1326:25
#66 0x10026eaa4 in top_level_1 src/keyboard.c:1129:5
#67 0x100361411 in internal_catch src/eval.c:1091:25
#68 0x100249135 in command_loop src/keyboard.c:1090:2
#69 0x100248f21 in recursive_edit_1 src/keyboard.c:697:9
#70 0x100249480 in Frecursive_edit src/keyboard.c:768:3
#71 0x1002453a1 in main src/emacs.c:1687:3
#72 0x7fffae305234 in start (/usr/lib/system/libdyld.dylib:x86_64+0x5234)
Address 0x7fff5fbfa690 is located in stack of thread T0 at offset 336 in frame
#0 0x1003eabbf in connect_network_socket src/process.c:3307
This frame has 10 object(s):
[32, 36) 'xerrno'
[48, 52) 'family'
[64, 68) 'optval'
[80, 96) 'sa1'
[112, 116) 'len1'
[128, 132) 'len'
[144, 272) 'fdset'
[304, 308) 'rfamily'
[320, 336) 'sa1261' <== Memory access at offset 336 overflows this variable
[352, 356) 'len1262'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow src/process.c:2497:34 in conv_sockaddr_to_lisp
Shadow bytes around the buggy address:
0x1fffebf7f480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1fffebf7f490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1fffebf7f4a0: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 04 f2 04 f2
0x1fffebf7f4b0: 04 f2 00 00 f2 f2 04 f2 04 f2 00 00 00 00 00 00
0x1fffebf7f4c0: 00 00 00 00 00 00 00 00 00 00 f2 f2 f2 f2 04 f2
=>0x1fffebf7f4d0: 00 00[f2]f2 04 f3 f3 f3 00 00 00 00 00 00 00 00
0x1fffebf7f4e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1fffebf7f4f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1fffebf7f500: 00 00 00 00 f1 f1 f1 f1 00 00 05 f2 f2 f2 f2 f2
0x1fffebf7f510: 04 f2 00 f2 f2 f2 00 00 00 00 00 00 f2 f2 f2 f2
0x1fffebf7f520: 00 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00 f3 f3
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
==31024==ABORTING
The problem is here:
struct sockaddr_in sa1;
socklen_t len1 = sizeof (sa1);
if (getsockname (s, (struct sockaddr *)&sa1, &len1) == 0)
contact = Fplist_put (contact, QClocal,
conv_sockaddr_to_lisp ((struct sockaddr *)&sa1, len1));
sockaddr_in is too small for IPv6 addresses, so getsockname doesn't fill it out completely. But conv_sockaddr_to_lisp only looks at the address family and attempts to read out the entire IPv6 address, reading past the sa1 variable memory. Thus this needs to be sockaddr_storage, which is guaranteed to be large enough for all supported addresses.
Probably there should also be an eassert(len1 <= sizeof sa1) after the call to getsockname, just to make sure.