Bug 395 - a minor "information leak" in refused response
a minor "information leak" in refused response
Status: RESOLVED FIXED
Product: unbound
Classification: Unclassified
Component: server
unspecified
All All
: P5 normal
Assigned To: unbound team
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-28 00:06 CEST by jinmei
Modified: 2011-06-30 11:44 CEST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description jinmei 2011-06-28 00:06:21 CEST
(I tested this for 1.4.10 and it's not listed in the "Version" menu, so
I chose "unspecified" in the menu.)

I've noticed a minor information leak in unbound: when returning a
"REFUSED" response due to "access-control" ACL, unbound could disclose
the QID (as well as some other parts of the first 4 bytes) of a
previous response.

At the beginning of daemon/worker.c:worker_handle_request() unbound
check its ACL, and the check results in "refuse", it returns a REFUSED
response without doing any validation on the packet (e.g. even whether
the incoming packet has a sufficient length for a complete DNS
message):

int 
worker_handle_request(struct comm_point* c, void* arg, int error,
	struct comm_reply* repinfo)
{
...
	} else if(acl == acl_refuse) {
		ldns_buffer_set_limit(c->buffer, LDNS_HEADER_SIZE);
		ldns_buffer_write_at(c->buffer, 4, 
			(uint8_t*)"\0\0\0\0\0\0\0\0", 8);
...

And, as shown in the code, it leaves the first 4 bytes unchanged (QR
and RCODE will be reset in subsequent lines, but other parts of 4
bytes aren't modified).  Furthermore, this buffer is reused for
subsequent incoming packets without any cleanup.

So, if someone sends a UDP packet with 0 length that would result in
"refused", the first 4 bytes (except QR and RCODE) of the response
will be the same as those used for the previous response.  In
particular, this means one can know the QID used by one of other
clients of unbound by sending such a crafted packet (and having it
refused, which I guess is relatively easy because today's recursive
servers are often run with an ACL that refuses queries except for a
limited set of addresses).

I confirmed my theory by actually sending 0-length UDP packets to
unbound and observing the first 4 bytes of the responses. (I could
attach sample test code to reproduce that, but I believe it's quite
straightforward and you can easily reproduce it yourself).

I personally don't think this matters much, but someone may consider
it a kind of security incident, so I'm reporting it here.  I'd leave
it to you whether to do anything about this.

And, in case you think it has some severity, I have to apologize: I
already mentioned the behavior of unbound that could lead to the above
scenario in a public list:
https://lists.isc.org/pipermail/bind10-dev/2011-June/002418.html
and you may want to change it sooner than later.

Finally, I noticed a minor issue in the top page for bug reports
(http://www.nlnetlabs.nl/labs/bugs/) when I tried to file this report:
it says: "Here you can enter a new bug report for NSD, drill or ldns."
It doesn't include unbound (although you may be as confident as
Dr. Knuth and intentionally omitted unbound from the list, though:-).
Comment 1 Wouter Wijngaards 2011-06-28 10:24:08 CEST
Hi Jinmei,

That is a good find, it does check the length of packets but only after the ACL check.  Guessing the random ID bits of another request, does not allow you to spoof to unbound, but would be bad for the security of that request, but you do not know the qname of it, nor the port number.  You are correct that it needs a fix sooner than later, but we are in freeze for RC3 of 1.4.11 release, that should happen later this week.  At this point a release for this fix becomes possible.

I think the correct fix is to reset the length to the original length (after putting the response bits in it), so the response is not bigger than the query.

The bugzilla line needs to be fixed, I am not dr.Knuth :-)
But it has a lower priority.

Thank you very much for finding this (obscure) bug and understanding its import and reporting it.

Best regards, Wouter
Comment 2 jinmei 2011-06-29 10:50:50 CEST
> That is a good find, it does check the length of packets but only
> after the ACL check.  Guessing the random ID bits of another
> request, does not allow you to spoof to unbound, but would be bad
> for the security of that request, but you do not know the qname of
> it, nor the port number.  You are correct that it needs a fix sooner
> than later, but we are in freeze for RC3 of 1.4.11 release, that
> should happen later this week.  At this point a release for this fix
> becomes possible.

That's fine.  I don't have any stake in this problem or have a strong
opinion on whether/how/when to fix it, so it's completely up to you.

> Thank you very much for finding this (obscure) bug and understanding
> its import and reporting it.

You're welcome, I'm glad if it was useful.
Comment 3 Wouter Wijngaards 2011-06-30 11:44:08 CEST
Hi Jinmei,

Fixed in r2444.

After some thought, the correct fix is to determine to drop messages too short or too malformed or give a REFUSED error.

Thanks for the report,
   Wouter