Inter-Process Vulnerabilities
==============================
Although we focused our audit on local and remote vulnerabilities,
nevertheless we identified four classes of common inter-process
vulnerabilities in OpenSMTPD:
- Out-of-bounds memory read: one process X receives a specific type of
structure (or structured data) from another process Y, but does not
check that process Y actually sent enough data; or, checks are
performed, but incorrectly (e.g., because of an integer-wrap).
- Indirect information leak: if such an out-of-bounds memory read
survives (i.e., X does not segfault), and if the partial structure
received by X is sent back to Y as a complete structure, information
from the memory of process X is leaked to process Y.
- Direct information leak: a structure sent by one process to another
process contains uninitialized fields (e.g., union fields, or large
string buffers that are only partially strlcpy()ed to), thus leaking
information from the memory of the sending process to the receiving
process.
- Out-of-bounds memory write: one process receives data from another
process and copies it into a buffer without checking that it actually
fits; or, a structure is received and its contents (e.g., size fields)
are trusted without checks.
------------------------------
In parent_imsg(), case IMSG_LKA_OPEN_FORWARD:
169 static void
170 parent_imsg(struct mproc *p, struct imsg *imsg)
171 {
172 struct forward_req *fwreq;
...
185 case IMSG_LKA_OPEN_FORWARD:
186 fwreq = imsg->data;
...
196 m_compose(p, IMSG_LKA_OPEN_FORWARD, 0, 0, fd,
197 fwreq, sizeof *fwreq);
- Indirect information leak: fwreq.
------------------------------
In queue_imsg(), case IMSG_SCHED_ENVELOPE_BOUNCE:
64 static void
65 queue_imsg(struct mproc *p, struct imsg *imsg)
66 {
..
68 struct bounce_req_msg *req_bounce;
..
238 case IMSG_SCHED_ENVELOPE_BOUNCE:
239 req_bounce = imsg->data;
...
250 queue_bounce(&evp, &req_bounce->bounce);
...
512 static void
513 queue_bounce(struct envelope *e, struct delivery_bounce *d)
514 {
515 struct envelope b;
...
519 b.agent.bounce = *d;
...
543 m_create(p_scheduler, IMSG_QUEUE_ENVELOPE_SUBMIT, 0, 0, -1);
544 m_add_envelope(p_scheduler, &b);
545 m_close(p_scheduler);
- Indirect information leak: req_bounce->bounce.
------------------------------
In control_imsg(), cases IMSG_STAT_INCREMENT, IMSG_STAT_DECREMENT,
IMSG_STAT_SET:
82 static void
83 control_imsg(struct mproc *p, struct imsg *imsg)
84 {
..
86 struct stat_value val;
87 struct msg m;
88 const char *key;
89 const void *data;
90 size_t sz;
..
165 case IMSG_STAT_SET:
166 m_msg(&m, imsg);
167 m_get_string(&m, &key);
168 m_get_data(&m, &data, &sz);
169 m_end(&m);
170 memmove(&val, data, sz);
- Out-of-bounds memory write (stack-based buffer overflow): in the call
to memmove(), the sz returned by m_get_data() is blindly trusted to be
equal to the size of the stack-based val structure (CVE-2015-ABCD).
------------------------------
In mda_imsg(), case IMSG_MDA_LOOKUP_USERINFO:
114 void
115 mda_imsg(struct mproc *p, struct imsg *imsg)
116 {
...
124 const void *data;
...
127 size_t sz;
...
134 case IMSG_MDA_LOOKUP_USERINFO:
135 m_msg(&m, imsg);
136 m_get_id(&m, &reqid);
137 m_get_int(&m, (int *)&status);
138 if (status == LKA_OK)
139 m_get_data(&m, &data, &sz);
140 m_end(&m);
...
144 if (status == LKA_TEMPFAIL)
...
148 else if (status == LKA_PERMFAIL)
...
152 else {
153 memmove(&u->userinfo, data, sz);
- Out-of-bounds memory write (heap-based buffer overflow): in the call
to memmove(), the sz returned by m_get_data() is blindly trusted to be
equal to the size of the heap-based u->userinfo structure
(CVE-2015-ABCD).
------------------------------
In mta_start_tls():
1501 static void
1502 mta_start_tls(struct mta_session *s)
1503 {
1504 struct ca_cert_req_msg req_ca_cert;
1505 const char *certname;
1506
1507 if (s->relay->pki_name)
1508 certname = s->relay->pki_name;
1509 else
1510 certname = s->helo;
1511
1512 req_ca_cert.reqid = s->id;
1513 (void)strlcpy(req_ca_cert.name
1514 m_compose(p_lka, IMSG_MTA_SSL_INIT, 0, 0, -1,
1515 &req_ca_cert, sizeof(req_ca_cert));
- Direct information leak: req_ca_cert.name is not fully initialized by
strlcpy() (unlike strncpy(), strlcpy() does not fill its destination
buffer with additional null-bytes), and therefore still contains
sensitive information from the stack.
------------------------------
In mta_session_imsg(), case IMSG_MTA_SSL_INIT:
253 struct ca_cert_resp_msg *resp_ca_cert;
...
314 case IMSG_MTA_SSL_INIT:
315 resp_ca_cert = imsg->data;
...
336 resp_ca_cert = xmemdup(imsg->data, sizeof *resp_ca_cert, "mta:ca_cert");
337 resp_ca_cert->cert = xstrdup((char *)imsg->data +
338 sizeof *resp_ca_cert, "mta:ca_cert");
...
343 ssl = ssl_mta_init(pkiname,
344 resp_ca_cert->cert, resp_ca_cert->cert_len);
...
349 explicit_bzero(resp_ca_cert->
- Out-of-bounds memory read: resp_ca_cert.
- Out-of-bounds memory read: the string passed to xstrdup() is not
guaranteed to be null-terminated.
- Out-of-bounds memory read: in the call to ssl_mta_init(),
resp_ca_cert->cert_len is blindly trusted to be equal to
resp_ca_cert->cert's xstrdup()ed length.
- Out-of-bounds memory write: in the call to explicit_bzero(),
resp_ca_cert->cert_len is blindly trusted to be equal to
resp_ca_cert->cert's xstrdup()ed length (CVE-2015-ABCD).
------------------------------
In mta_session_imsg(), case IMSG_MTA_SSL_VERIFY:
252 struct ca_vrfy_resp_msg *resp_ca_vrfy;
...
354 case IMSG_MTA_SSL_VERIFY:
355 resp_ca_vrfy = imsg->data;
356 s = mta_tree_pop(&wait_ssl_verify, resp_ca_vrfy->reqid);
...
360 if (resp_ca_vrfy->status == CA_OK)
- Out-of-bounds memory read: resp_ca_vrfy.
------------------------------
In smtp_session_imsg(), case IMSG_SMTP_SSL_INIT:
551 struct ca_cert_resp_msg *resp_ca_cert;
...
831 case IMSG_SMTP_SSL_INIT:
832 resp_ca_cert = imsg->data;
...
842 resp_ca_cert = xmemdup(imsg->data, sizeof *resp_ca_cert, "smtp:ca_cert");
...
845 resp_ca_cert->cert = xstrdup((char *)imsg->data +
846 sizeof *resp_ca_cert, "smtp:ca_cert");
...
861 explicit_bzero(resp_ca_cert->
- Out-of-bounds memory read: resp_ca_cert.
- Out-of-bounds memory read: the string passed to xstrdup() is not
guaranteed to be null-terminated.
- Out-of-bounds memory write: in the call to explicit_bzero(),
resp_ca_cert->cert_len is blindly trusted to be equal to
resp_ca_cert->cert's xstrdup()ed length (CVE-2015-ABCD).
------------------------------
In smtp_session_imsg(), case IMSG_SMTP_SSL_VERIFY:
552 struct ca_vrfy_resp_msg *resp_ca_vrfy;
...
866 case IMSG_SMTP_SSL_VERIFY:
867 resp_ca_vrfy = imsg->data;
868 s = tree_xpop(&wait_ssl_verify, resp_ca_vrfy->reqid);
869
870 if (resp_ca_vrfy->status == CA_OK)
- Out-of-bounds memory read: resp_ca_vrfy.
------------------------------
In smtp_mfa_response() and smtp_io(), cases IMSG_SMTP_REQ_CONNECT and
IO_LOWAT, respectively:
888 static void
889 smtp_mfa_response(struct smtp_session *s, int msg, int status, uint32_t code,
890 const char *line)
891 {
892 struct ca_cert_req_msg req_ca_cert;
...
905 case IMSG_SMTP_REQ_CONNECT:
...
915 if (s->listener->pki_name[0])
916 (void)strlcpy(req_ca_cert.name
917 sizeof req_ca_cert.name);
918 else
919 (void)strlcpy(req_ca_cert.name
920 sizeof req_ca_cert.name);
921 m_compose(p_lka, IMSG_SMTP_SSL_INIT, 0, 0, -1,
922 &req_ca_cert, sizeof(req_ca_cert));
- Direct information leak: req_ca_cert.name is not fully initialized by
strlcpy(), and therefore still contains sensitive information from the
stack.
------------------------------
In lka_imsg(), case IMSG_{SMTP,MTA}_SSL_INIT:
struct ca_cert_resp_msg {
uint64_t reqid;
enum ca_resp_status status;
char *cert;
off_t cert_len;
};
63 static void
64 lka_imsg(struct mproc *p, struct imsg *imsg)
65 {
..
75 struct ca_cert_resp_msg resp_ca_cert;
..
126 case IMSG_SMTP_SSL_INIT:
127 req_ca_cert = imsg->data;
128 resp_ca_cert.reqid = req_ca_cert->reqid;
...
133 if (pki == NULL) {
134 resp_ca_cert.status = CA_FAIL;
135 m_compose(p, IMSG_SMTP_SSL_INIT, 0, 0, -1, &resp_ca_cert,
136 sizeof(resp_ca_cert));
137 return;
138 }
- Out-of-bounds memory read: req_ca_cert.
- Direct information leak: resp_ca_cert's cert and cert_len fields.
------------------------------
In ca_imsg(), case IMSG_CA_PRIV{ENC,DEC}:
292 case IMSG_CA_PRIVENC:
293 case IMSG_CA_PRIVDEC:
294 m_msg(&m, imsg);
295 m_get_id(&m, &id);
296 m_get_string(&m, &pkiname);
297 m_get_data(&m, &from, &flen);
298 m_get_size(&m, &tlen);
299 m_get_size(&m, &padding);
300 m_end(&m);
...
307 if ((to = calloc(1, tlen)) == NULL)
308 fatalx("ca_imsg: calloc");
309
310 switch (imsg->hdr.type) {
311 case IMSG_CA_PRIVENC:
312 ret = RSA_private_encrypt(flen, from, to, rsa,
313 padding);
314 break;
315 case IMSG_CA_PRIVDEC:
316 ret = RSA_private_decrypt(flen, from, to, rsa,
317 padding);
318 break;
319 }
- Out-of-bounds memory write (heap-based buffer overflow): in the call
to RSA_private_{enc,dec}rypt(), the size (tlen) of the destination
buffer (to) is blindly trusted to be equal to RSA_size(rsa)
(CVE-2015-ABCD).
------------------------------
In m_get_typed_sized():
493 static inline void
494 m_get_typed_sized(struct msg *m, uint8_t type, const void **dst, size_t *sz)
495 {
496 if (m->pos + 1 + sizeof(*sz) > m->end)
497 m_error("msg too short");
498 if (*m->pos != type)
499 m_error("msg bad type");
500 memmove(sz, m->pos + 1, sizeof(*sz));
501 m->pos += sizeof(sz) + 1;
502 if (m->pos + *sz > m->end)
503 m_error("msg too short");
504 *dst = m->pos;
505 m->pos += *sz;
506 }
- Out-of-bounds memory read: *sz, the amount of data allegedly received,
is read directly from the wire and sanity-checked, but large *sz
values can integer-wrap the check.
------------------------------
In m_get_sockaddr():
705 void
706 m_get_sockaddr(struct msg *m, struct sockaddr *sa)
707 {
708 size_t s;
709 const void *d;
710
711 m_get_typed_sized(m, M_SOCKADDR, &d, &s);
712 memmove(sa, d, s);
713 }
- Out-of-bounds memory write (buffer overflow): in the call to
memmove(), the size s of the data d returned by m_get_typed_sized() is
blindly trusted to be equal to the size of the sockaddr structure sa
(CVE-2015-ABCD).
==============================
Miscellaneous Bugs
==============================
------------------------------
In mta_imsg(), case IMSG_CTL_RESUME_ROUTE, if u64 is 0 ("resuming all
routes"), mta_route_unref() may eventually free() route, which is then
used-after-free by SPLAY_NEXT() in SPLAY_FOREACH() (there is a SAFE
version of most FOREACH macros, but no SPLAY_FOREACH_SAFE()):
419 case IMSG_CTL_RESUME_ROUTE:
420 u64 = *((uint64_t *)imsg->data);
...
426 SPLAY_FOREACH(route, mta_route_tree, &routes) {
427 if (u64 && route->id != u64)
428 continue;
429
430 if (route->flags & ROUTE_DISABLED) {
...
441 mta_route_unref(route); /* from mta_route_disable */
442 }
443
444 if (u64)
445 break;
446 }
447 return;
------------------------------
In parent_sig_handler(), the cause pointer should always be initialized
to NULL before the calls to asprintf(), and the return value of these
calls should be checked (on OpenBSD, asprintf() will always reset the
cause pointer to NULL in case of a failure, but this behavior is
implementation-dependent):
351 static void
352 parent_sig_handler(int sig, short event, void *p)
353 {
...
357 char *cause;
...
365 do {
366 pid = waitpid(-1, &status, WNOHANG);
...
371 if (WIFSIGNALED(status)) {
...
373 asprintf(&cause, "terminated; signal %d",
374 WTERMSIG(status));
375 } else if (WIFEXITED(status)) {
376 if (WEXITSTATUS(status) != 0) {
...
378 asprintf(&cause, "exited abnormally");
379 } else
380 asprintf(&cause, "exited okay");
381 } else
382 fatalx("smtpd: unexpected cause of SIGCHLD");
...
442 free(cause);
443 } while (pid > 0 || (pid == -1 && errno == EINTR));
------------------------------
Code similar to the following appears several times in OpenSMTPD:
979 (void)strlcpy(sfn, "/tmp/smtpd.out.XXXXXXXXXXX", sizeof(sfn));
980 omode = umask(7077);
981 allout = mkstemp(sfn);
982 umask(omode);
983 if (allout < 0) {
...
991 return;
992 }
993 unlink(sfn);
But 7077 is decimal, not octal; in octal, 7077 is 015645. Luckily, the
call to mkstemp() that always follows uses mode 0600, which results in
the final mode 0000 (0600 & ~015645). This is not a security issue,
because these permissions are even more restrictive than those
originally intended.
------------------------------
In do_show_queue(), chdir(".") should rather be chdir("/"), because the
current working directory may be outside the chroot tree:
652 if (chroot(PATH_SPOOL) == -1 || chdir(".") == -1)
653 err(1, "%s", PATH_SPOOL);
However, this is not a security issue either: do_show_queue() is an
smtpctl functionality, only root is allowed to run it, and all
subsequent filesystem accesses begin with '/' anyway.
==============================
Acknowledgments
==============================
We would like to thank OpenSMTPD's developers for their cooperation,
professional work, and minute attention to every detail in our audit
report.
Komentarų nėra:
Rašyti komentarą