1 This is the xinetd-2.3.0 audit status. The audit has been performed
2 in order to make the Owl (http://www.openwall.com/Owl/) xinetd package
3 reasonably secure and of course with the hope that others will find
4 the results and patches useful as well.
6 Much of xinetd's logic is left unaudited (other than for generic bug
7 classes listed below). In particular, this applies to all network
10 To summarize the results, xinetd may be reasonably safe to use with
11 these patches, but the code remains far from clean and certain bugs
14 The format of this list is one item per line, with subitems indented.
15 If a line doesn't start with a '+', that item hasn't been completed
16 (audited and/or patched).
18 None of the PATCH'es are a part of xinetd-2.3.0; they will be in the
19 Owl package and hopefully will get incorporated into future versions
23 Solar Designer <solar@openwall.com>
25 +BUGS strx_* functions (danger: don't always NUL-terminate)
26 +PATCH bug: shouldn't write NUL when len <= 0
27 +PATCH may overflow with huge precision values (bad for format bugs)
28 +BUGS strx_* calls: some assume NUL-termination, but none look dangerous
29 +PATCH __xlog_explain_errno forgets to update size
30 +PATCH child.c: child_process may not NUL-terminate name
31 +PATCH init.c: syscall_failed may not NUL-terminate err
32 +PATCH shutdown.c: safe, but should use print_buf_size-1
33 +PATCH signals.c: sig_name should use sizeof(signame_buf)-1
36 +OK strcat, strcpy calls (testsuite calls not checked)
38 +PATCH some inefficient, but correct
39 +BUGS memcpy, memmove, bcopy calls
40 +PATCH addr.c: addrlist_dump() copies ipv6 address into ipv4 struct
41 +PATCH addr.c: host_addr() trusts hep->h_length from gethostbyname()
42 +PATCH parsers.c: redir_parser() wouldn't build/work when NO_INET_ATON
43 +PATCH parsers.c: redir_parser() wrongly relies on sizeof(he->h_addr)
44 +PATCH parsers.c: bind_parser() same as the above
45 the use of sizeof() is inconsistent (not always of dst arg)
46 +PATCH should change bcopy to memcpy/memmove as appropriate
47 +BUGS sio_memcopy calls
49 +BUGS siosup.c is too complicated in its handling of data sizes
50 +OK expand() is only called with old_size < new_size
51 +? buffer_setup() isn't obviously correct, but is safe
52 +OK __sio_extend_buffer is correct
53 +PATCH comment to sio.h: aux buffer is right below buf
54 +PATCH __sio_get_events may cause bufentries < 0 (->overflow)
55 +OK conn_setaddr calls: safe, but could use sizeof((cp)->co_remote_address)
58 +PATCH addr.c: explicit_mask() has single-byte overflow of saddr[]
60 +OK fprintf, sprintf, fscanf, sscanf
61 +OK syslog (but relies on %.*s for non-NUL-terminated buffers)
63 +OK svc_logprint, prepare_buffer
65 +PATCH intcommon.c: int_init() passes fmt as wrong arg
66 +PATCH (non-security) should never have '\n' in format
69 +OK ostimer.c: terminate
70 +PATCH xlog_write() is called on untrusted input w/o XLOG_NO_ERRNO
71 +OK ident -- looks mostly safe
72 +OK SIGALRM + longjmp() used safely:
73 +OK no static accesses, no stdio between START/STOP_TIMER
74 +OK immediate return on timeout (-> no clobbering issues)
75 +OK signal mask after longjmp() is unimportant
76 +PATCH could be safer to also reset SIGALRM handler
77 +PATCH verify_line() modifies buf, which is then logged on error
78 will log control chars from remote
80 +OK time, daytime, sensor: NO_FORK && stream (safe: FNDELAY)
81 +PATCH accept() return value never checked
82 +PATCH bad_port_check(): should deny all <1024 (53/udp is just as bad)
83 +PATCH stream_daytime writes to wrong fd when wait = yes (gets EPIPE)
84 +PATCH *_time sends sizeof(time_t): wrong on at least linux-alpha
85 xadmin_handler(): the command parser is unreliable (use sio?)
86 +BUGS record (shutdown.c)
87 connect_back may be used for portscanning of own machine
88 write() return values not checked
89 +PATCH will only handle traditional (obsolete) crypt(3) hashes
90 special.c: stream_shutdown() will log control chars from remote
91 intercept (int[.c]*, {tcp,udp}int.c) -- checked for generic bugs ONLY
92 tcpint.c: si_exit() may leave open fd from accept()
93 +BUGS signal races, longjmp clobbering
94 +BUGS __ostimer_interrupt:
95 +PATCH call_level should be volatile
96 +PATCH should use &ret_env, not (char *)ret_env (may differ)
97 +BUGS ret_env modified non-atomically (with 2 TIMER_LONGJMP's)
98 +PATCH should set have_env before and make it volatile
99 +PATCH may leave an altered signal mask on longjmp()
100 should fallback to plain longjmp in configure
101 +OK __ostimer_{add,remove} only called with signals blocked
102 +OK timer_s fields not volatile, safe due to block _calls_
103 +BUGS check uses of TIMER_LONGJMP flag
104 +BUGS confparse.c: get_conf() may jump out of malloc(), etc
105 no other uses, perhaps just disable CONF_TIMEOUT
106 +OK ident.c: mostly safe (see above)
107 +BUGS signals.c: bad_signal(): only on crashes, so not a big issue
108 +PATCH *count should be volatile to really avoid looping
109 does calls which may cause another SIGSEGV w/o a bug
110 +PATCH may leave an altered signal mask on longjmp()
111 +BUGS signals.c: general_handler()
112 sio and non-reentrant libc calls on unexpected signal
113 +BUGS signals.c: handle_signal(), my_handler(): events may be lost
114 my_handler may be re-entered, but M_SET isn't atomic:
115 should split ps.flags into int-per-flag
116 +OK main.c: main(): setjmp() placed in a way avoiding clobbering
117 +BUGS access.c: parent_access_control(), alrm_ena() (cps feature)
118 alrm_ena() may cause re-entry into syslog(), etc
119 SIGALRM handler may be never reset, or --
120 the handler and/or alarm may be reset for other needs
121 should use the timer queues, not OS timers
122 +BUGS int.c: int_sighandler(), intcommon.c: int_init()
123 int_sighandler() may cause re-entry into msg(), etc
124 installed for multiple signals, doesn't block
125 may interrupt msg() in main and cause re-entry
126 +PATCH redirect.c: redir_sigpipe() should use _exit(2), not exit(3)
127 +BUGS fd_set overflows
128 intcommon.c: sets INT_REMOTE(ip) w/o fd_set size check
129 {tcp,udp}int.c: si_mux(), di_mux() FD_SET w/o fd check
130 internals.c: socket_mask_copy w/o fd checks
131 main_loop(): select() on read_mask w/o fd checks
132 service.c: svc_activate(): could check ps.rws.mask_max here
133 (many files) all references to ps.rws.socket_mask are unchecked
134 redirect.c: redir_handler() no checks for rdfd, msfd
135 should use fd_grow similarly to OpenBSD inetd
136 +PATCH workaround: reduce RLIMIT_NOFILE to FD_SETSIZE
137 +BUGS __sio_descriptors overflows
138 +PATCH sio functions forget to check fd against n_descriptors
139 +BUGS get_fd_limit(), Smorefds()
140 +PATCH assume RLIMIT_NOFILE is small (not RLIM_INFINITY)
141 +OK orig_max_descriptors and max_descriptors are rlim_t, not int
142 +BUGS potential fd leaks to services
143 +PATCH init.c: setup_file_descriptors() relies on the rlimit only
144 returns from close() never checked -- fixed the worst one
145 +BUGS child.c: set_credentials()
146 +PATCH should fail if !ps.ros.is_superuser && user/group requested
147 +OK is otherwise fail-close and resets groups (fixed long ago)
148 +BUGS gethostby*, getaddrinfo (some are common with memcpy bugs)
149 +PATCH addr.c: host_addr() trusts hep->h_length from gethostbyname()
150 addr.c: host_addr() INET6 doesn't check res->ai_family
151 parsers.c: {redir,bind}_parser() don't check res->ai_family
152 +PATCH parsers.c: redir_parser() wrongly relies on sizeof(he->h_addr)
153 +PATCH parsers.c: bind_parser() same as the above
154 +PATCH getpwnam unnecessarily leaves password hashes in address space on BSD
156 +PATCH gcc format attributes
157 + build with gcc -Wall -Wcast-align (x86, alpha) -- mostly clean
158 +PATCH many unused vars with ipv6
159 +PATCH parsers.c, inet.c: stores strtol() to int, then needs long
160 +PATCH several format strings don't match arguments
161 + build with ccc -msg_enable level4 or higher
162 +PATCH CC= from configure doesn't get into Makefile's
163 lots of warnings (250 KB of output), the code is just not clean
164 +PATCH some really need fixing
165 - use wrapper functions around either strx_* or vsnprintf()?
166 + not a good idea, tested strx_* against snprintf instead
167 ? define strz_* wrappers around strx_*, which would always NUL-terminate
168 +PATCH atoi -> strtol with long to int overflow checks
170 should limit logging rate (= rate of permitted sessions, popa3d-like)
171 should drop privileges for ident lookups, builtins, and records
172 should have options to build --without-{ident,builtins,record,intercept}
173 should generate manpages accordingly