Always nul-terminate the result passed to evdns_server_add_ptr_reply

In dnsserv_resolved(), we carefully made a nul-terminated copy of the
answer in a PTR RESOLVED cell... then never used that nul-terminated
copy.  Ouch.

Surprisingly this one isn't as huge a security problem as it could be.
The only place where the input to dnsserv_resolved wasn't necessarily
nul-terminated was when it was called indirectly from relay.c with the
contents of a relay cell's payload.  If the end of the payload was
filled with junk, eventdns.c would take the strdup() of the name [This
part is bad; we might crash there if the cell is in a bad part of the
stack or the heap] and get a name of at least length
495[*]. eventdns.c then rejects any name of length over 255, so the
bogus data would be neither transmitted nor altered.

  [*] If the name was less than 495 bytes long, the client wouldn't
     actually be reading off the end of the cell.

Nonetheless this is a reasonably annoying bug.  Better fix it.

Found while looking at bug 2332, reported by doorss.  Bugfix on
0.2.0.1-alpha.
This commit is contained in:
Nick Mathewson 2011-01-10 16:18:32 -05:00
parent d43cba6c69
commit bd67b23fec
2 changed files with 5 additions and 1 deletions

4
changes/bug2332 Normal file
View File

@ -0,0 +1,4 @@
o Minor bugfixes
- Fix a bug with handling misformed replies to reverse DNS lookup
requests in DNSPort. Bugfix on Tor 0.2.0.1-alpha. Related to a bug
reported by doorss.

View File

@ -275,7 +275,7 @@ dnsserv_resolved(edge_connection_t *conn,
char *ans = tor_strndup(answer, answer_len); char *ans = tor_strndup(answer, answer_len);
evdns_server_request_add_ptr_reply(req, NULL, evdns_server_request_add_ptr_reply(req, NULL,
name, name,
(char*)answer, ttl); ans, ttl);
tor_free(ans); tor_free(ans);
} else if (answer_type == RESOLVED_TYPE_ERROR) { } else if (answer_type == RESOLVED_TYPE_ERROR) {
err = DNS_ERR_NOTEXIST; err = DNS_ERR_NOTEXIST;