Bug 766

Summary: "visudo -c" may crash in addr_matches_if()
Product: Sudo Reporter: Nicolas Iooss <nicolas.iooss.2010_sudo>
Component: VisudoAssignee: Todd C. Miller <Todd.Miller>
Status: RESOLVED FIXED    
Severity: normal    
Priority: low    
Version: 1.8.19   
Hardware: PC   
OS: Linux   

Description Nicolas Iooss 2017-01-07 15:32:51 MST
The following statements in a sudoers file make visudo checker crash:

    Host_Alias LOCAL = 127.0.0.1
    Defaults@LOCAL insults

Here is a gdb session which highlights where the problem might come from:

$ gdb -q -ex r --args ./plugins/sudoers/.libs/lt-visudo -cf <(printf 'Host_Alias LOCAL = 127.0.0.1\nDefaults@LOCAL insults\n')                                           
Reading symbols from ./plugins/sudoers/.libs/lt-visudo...done.
Starting program: /home/nicolas/temporary_projects/src/sudo/plugins/sudoers/.libs/lt-visudo -cf /proc/self/fd/11

Program received signal SIGSEGV, Segmentation fault.
0x000002aaaaafc62d in addr_matches_if (n=0x2aaaad3b100 "127.0.0.1") at ./match_addr.c:69
69	    SLIST_FOREACH(ifp, get_interfaces(), entries) {
   0x000002aaaaafc5f0 <addr_matches+4912>:	48 8d a4 24 68 ff ff ff	lea    -0x98(%rsp),%rsp
   0x000002aaaaafc5f8 <addr_matches+4920>:	48 89 14 24	mov    %rdx,(%rsp)
   0x000002aaaaafc5fc <addr_matches+4924>:	48 89 4c 24 08	mov    %rcx,0x8(%rsp)
   0x000002aaaaafc601 <addr_matches+4929>:	48 89 44 24 10	mov    %rax,0x10(%rsp)
   0x000002aaaaafc606 <addr_matches+4934>:	48 c7 c1 a2 70 00 00	mov    $0x70a2,%rcx
   0x000002aaaaafc60d <addr_matches+4941>:	e8 e6 1d 00 00	callq  0x2aaaaafe3f8 <__afl_maybe_log>
   0x000002aaaaafc612 <addr_matches+4946>:	48 8b 44 24 10	mov    0x10(%rsp),%rax
   0x000002aaaaafc617 <addr_matches+4951>:	48 8b 4c 24 08	mov    0x8(%rsp),%rcx
   0x000002aaaaafc61c <addr_matches+4956>:	48 8b 14 24	mov    (%rsp),%rdx
   0x000002aaaaafc620 <addr_matches+4960>:	48 8d a4 24 98 00 00 00	lea    0x98(%rsp),%rsp
   0x000002aaaaafc628 <addr_matches+4968>:	e8 33 f0 fc ff	callq  0x2aaaaacb660 <get_interfaces>
=> 0x000002aaaaafc62d <addr_matches+4973>:	48 8b 10	mov    (%rax),%rdx
   0x000002aaaaafc630 <addr_matches+4976>:	48 85 d2	test   %rdx,%rdx
   0x000002aaaaafc633 <addr_matches+4979>:	0f 84 55 0a 00 00	je     0x2aaaaafd08e <addr_matches+7630>
   0x000002aaaaafc639 <addr_matches+4985>:	0f 1f 00	nopl   (%rax)
   0x000002aaaaafc63c <addr_matches+4988>:	48 8d a4 24 68 ff ff ff	lea    -0x98(%rsp),%rsp
   0x000002aaaaafc644 <addr_matches+4996>:	48 89 14 24	mov    %rdx,(%rsp)
   0x000002aaaaafc648 <addr_matches+5000>:	48 89 4c 24 08	mov    %rcx,0x8(%rsp)
   0x000002aaaaafc64d <addr_matches+5005>:	48 89 44 24 10	mov    %rax,0x10(%rsp)
   0x000002aaaaafc652 <addr_matches+5010>:	48 c7 c1 f1 76 00 00	mov    $0x76f1,%rcx
   0x000002aaaaafc659 <addr_matches+5017>:	e8 9a 1d 00 00	callq  0x2aaaaafe3f8 <__afl_maybe_log>
   0x000002aaaaafc65e <addr_matches+5022>:	48 8b 44 24 10	mov    0x10(%rsp),%rax
   0x000002aaaaafc663 <addr_matches+5027>:	48 8b 4c 24 08	mov    0x8(%rsp),%rcx
   0x000002aaaaafc668 <addr_matches+5032>:	48 8b 14 24	mov    (%rsp),%rdx
   0x000002aaaaafc66c <addr_matches+5036>:	48 8d a4 24 98 00 00 00	lea    0x98(%rsp),%rsp
(gdb) bt
#0  0x000002aaaaafc62d in addr_matches_if (n=0x2aaaad3b100 "127.0.0.1") at ./match_addr.c:69
#1  addr_matches (n=0x2aaaad3b100 "127.0.0.1") at ./match_addr.c:203
#2  0x000002aaaaafad59 in hostlist_matches (pw=pw@entry=0x2aaaad3d888, list=list@entry=0x2aaaad3b138) at ./match.c:281
#3  0x000002aaaaafae08 in hostlist_matches (pw=0x2aaaad3d888, list=<optimized out>) at ./match.c:286
#4  0x000002aaaaae4583 in default_binding_matches (what=<optimized out>, d=<optimized out>, d=<optimized out>) at ./defaults.c:678
#5  0x000002aaaaaeab6c in update_defaults (what=7, quiet=<optimized out>) at ./defaults.c:735
#6  0x000002aaaaab09eb in check_syntax (oldperms=true, strict=<optimized out>, quiet=<optimized out>, sudoers_file=0x3ffffffdde8 "/proc/self/fd/11") at ./visudo.c:967
#7  main (argc=<optimized out>, argv=<optimized out>) at ./visudo.c:235
(gdb) list
64		family = AF_INET;
65	    } else {
66		debug_return_bool(false);
67	    }
68	
69	    SLIST_FOREACH(ifp, get_interfaces(), entries) {
70		if (ifp->family != family)
71		    continue;
72		switch (family) {
73		    case AF_INET:
(gdb) p get_interfaces()
$1 = (struct interface *) 0x0


Function get_interfaces() always returns NULL in plugins/sudoers/visudo.c but its result is used without any check in addr_matches_if().
Comment 1 Todd C. Miller 2017-01-07 19:51:30 MST
Fixed by https://www.sudo.ws/repos/sudo/rev/ff9001f126b5
Comment 2 Todd C. Miller 2017-01-14 06:24:17 MST
Fixed in sudo 1.8.19p2, available now.