google / oss-fuzz

OSS-Fuzz - continuous fuzzing for open source software.
https://google.github.io/oss-fuzz
Apache License 2.0
10.32k stars 2.2k forks source link

"Arbitrary domain name resolution" is too vague to be useful as a bug description #10205

Open guyharris opened 1 year ago

guyharris commented 1 year ago

In infra/experimental/SystemSan/inspect_dns.cpp, the (experimental) "system sanitizer" (SystemSan) catches sendmsg() calls to port 53 of DNS queries in which "the top level domain is only one character and ... there is more than just the TLD", to quote the comment in the code, and reports that as an "Arbitrary domain name resolution".

Unless "arbitrary" or "arbitrary domain name resolution" is used as a term of art to refer to that precise problem, it's a very vague term that could mean "any DNS query that sends a host name other than www.example.com to a resolver is an arbitrary domain name resolution" to "any DNS query that sends a host name of y0u.h4v3.b33n.pwn3d.com to a resolver is an arbitrary domain name resolution" or "any DNS query that sends a host name with characters other than (A-Z), (a-z), (0-9), -, and periods between (non-empty) labels is an arbitrary domain name resolution". "Any DNS query that sends a host name in which the final label is one character long and there are additional labels" is not the most obvious meaning of "an arbitrary domain name resolution".

A more precise description is called for here. (And if additional checks are added, more precise descriptions of them would be called for as well.)

(In addition, it might be helpful to indicate precisely what is problematic about such a domain name resolution. Perhaps I missed it, but nothing in the Injection Attacks Reloaded: Tunnelling Malicious Payloads over DNS paper from the 30th USENIX Security Symposium appears to mention that. Perhaps there's some other paper or other document that mentions it?)

jonathanmetzman commented 1 year ago

Thank you for this detailed issue. CC @catenacyber I think it's a symptom of SSRF that's why we want to find it. But I agree it's not as clear cut as an issue as use-after-free (no application should do the latter, but curl should resolve arbitrary domains)

catenacyber commented 1 year ago

Thanks Guy for posting here

@jonathanmetzman what is the status of SystemSanitizer options per fuzz target ? libpcap does DNS resolutions for the BPF filter : host 8.8.8.8 or host whatever.z will resolve whatever.z and filter on tis IP address. So, the target fuzz_filter will trigger SystemSanitizer arbitrary DNS resolution, and we would like to disable this rule for this fuzz target

jonathanmetzman commented 1 year ago

I'd say it's deprioritized for the moment.

guyharris commented 1 year ago

libpcap does DNS resolutions for the BPF filter : host 8.8.8.8 or host whatever.z will resolve whatever.z and filter on tis IP address. So, the target fuzz_filter will trigger SystemSanitizer arbitrary DNS resolution, and we would like to disable this rule for this fuzz target

Why is resolving "whatever.z" an problem but resolving "whatever.zz", for example, not a problem?

catenacyber commented 1 year ago

Why is resolving "whatever.z" an problem but resolving "whatever.zz", for example, not a problem?

Because there is none 1-letter top level domain, and it is a simple heuristic.

guyharris commented 1 year ago

Because there is none 1-letter top level domain, and it is a simple heuristic.

What is the behavior that this heuristic is attempting to identify, and why is that behavior something that any software should avoid?

catenacyber commented 1 year ago

What is the behavior that this heuristic is attempting to identify

Arbitrary DNS resolutions

why is that behavior something that any software should avoid?

I do not think that any software should avoid to do arbitrary DNS resolutions As Jonathan said, curl is perfectly legit to do it. I think libpcap filter is ok as well. But java stdlib ipv6 parsing, or some log4j, and most oss-fuzz targets should not lead to an arbitrary DNS resolution.

guyharris commented 1 year ago

Arbitrary DNS resolutions

Which, as I said, is too vague to be useful. As far as I can tell, what it means here is "DNS resolution that does not perform any form of whitelisting or blacklisting", without specifying which forms would prevent access to untrusted hosts or some other form of insecure behavior rather than just randomly preventing useful resolutions and not blocking inappropriate resolutions.

I do not think that any software should avoid to do arbitrary DNS resolutions As Jonathan said, curl is perfectly legit to do it. I think libpcap filter is ok as well.

Then perhaps this test should be opt-in, rather than opt-out.

But java stdlib ipv6 parsing

What strings should, and what strings shouldn't, it attempt to resolve?

or some log4j,

Presumably this is referring to something other than log4shell, as incautious host name resolution doesn't seem to be a significant part of that problem - incautious choice of what strings should be subject to ${whatever} expansion and what strings should be passed on without interpretation (other than, perhaps, vetting for being valid UTF-8) seems to be at the heart of that problem.

(I.e., it's like

foo(char *string)
{
    ...

    printf(string);
}

only, given the stuff supported by log4j ${whatever} expansion, vastly more dangerous.)

So what strings should, and what strings shouldn't, it attempt to resolve?

most oss-fuzz targets

"Most"? How many targets are there, and how many have you determined shouldn't resolve arbitrary names, how many have you determined are OK to resolve arbitrary names, and how many have not been investigated to see whether they should be checked?

And what if some forms of input should have substrings resolved as host names without whitelisting or blacklisting, some forms of input should have substrings resolved but with whitelisting or blacklistings, and some should have any of their content be resolved as host names?

guyharris commented 1 year ago

Or if what "arbitrary DNS resolution" means is "the code extracts domain names from the input and attempts to resolve them", rather than "this code tries to resolve some domain names, whatever their source, without filtering them through some whitelisting/blacklisting criterion first", with the idea being to throw a known-invalid (at least until ".z" or whatever becomes a TLD) domain name into the input and seeing whether it, or some variant of it, shows up in a DNS request being sent out, then say that, rather than just saying "arbitrary domain name resolution"...

...and exclude from this test all code where this is intended behavior. Presumably if, for example, the nslookup, host, or dig command were to be run through oss-fuzz, this particular check would not be enabled.

evverx commented 1 year ago

Then perhaps this test should be opt-in, rather than opt-out.

FWIW I agree that experimental sanitizers should be opt-in by default. As far as I can remember it was already brought up when allocation failures and arbitrary read-writes were discussed.

catenacyber commented 1 year ago

Then perhaps this test should be opt-in, rather than opt-out.

There is not even the opt-out yet

"Most"?

IIRC, 8 projects out of 1078 do arbitrary names resolutions (and seemed legit, except the java stdlib bug found through the jetty project)

it was already brought up when allocation failures and arbitrary read-writes were discussed.

Allocation failures is proposed as an opt-in, not merged yet

evverx commented 1 year ago

Allocation failures is proposed as an opt-in, not merged yet

It isn't merged yet but if it was and, say, some projects opted in bug reports would be received by maintainers/contributors who could be unaware of that so those bug reports should be comprehensible (and that's what this issue is about as far as I understand). Even if they were made comprehensible I don't think at this point it's possible to reproduce issues found by experimental sanitizers locally (at least infra/helper.py doesn't seem to support those sanitizers).

Anyway, what I'm trying to say is that experimental sanitizers in general should be integrated into OSS-Fuzz a bit better in terms of opting-in/opting-out, documentation, local builds and so on.