SystemSan: arbitrary DNS resolution detection

[catenacyber]

cc @oliverchang @jonathanmetzman

Here is a new bug class for SystemSan : arbitrary DNS resolution (like in log4j)

What do you think about it ?

[catenacyber]

Rebased and fixed the conflict on #includes, and force-pushed

Friendly ping on this ?

cc @alan32liu

[oliverchang]

Rebased and fixed the conflict on #includes, and force-pushed

Friendly ping on this ?

cc @alan32liu

Thanks for this change. The idea is nice, and similar to what we had in mind, but the code style will need a bit of improvement to make this maintainable.

At a minimum we'd like to see the DNS parsing have proper structs/classes defined etc and in its own file. That said, are there any existing libraries out there we can also use for this instead of trying to roll our own?

[catenacyber]

That said, are there any existing libraries out there we can also use for this instead of trying to roll our own?

Interesting point.
That depends on what you prefer : external dependencies or maintain your own code

The DNS parsing done here is specific to the case we are interested in (simple domain query)
And I thought you would not want any dependencies for SystemSan.
So, what do you prefer ?

[oliverchang]

That said, are there any existing libraries out there we can also use for this instead of trying to roll our own?

Interesting point. That depends on what you prefer : external dependencies or maintain your own code

The DNS parsing done here is specific to the case we are interested in (simple domain query) And I thought you would not want any dependencies for SystemSan. So, what do you prefer ?

That would depend on what the dependency looks like and its size :) e.g. If the dependency is a small header-only library that's well tested, that would be a good candidate.

Otherwise, we'd need to clean up this current implementation a bit before we can accept it.

[catenacyber]

Here is a new version in its own file

I did not find any good candidate at first sight for a third party dns parsing library.
My best proposal would be https://docs.rs/dns-parser/latest/dns_parser/
Would rust suit you ?
The only dns parsing library I see on oss-fuzz is go-dns, which is in Golang... The other projects are full-fletched dns servers...

If we go for rolling our own, do I understand correctly the following ?
inspect_for_arbitrary_dns_pkt should create structures/classes such as a DNS Header (cf https://docs.rs/dns-parser/latest/dns_parser/struct.Header.html), and then check that the questions field of this structure is equal to 1 instead of directly doing if (uint8_t(data[4]) != 0 || uint8_t(data[5]) != 1) {

[catenacyber]

Friendly ping on this ?

[catenacyber]

@alan32liu what do you think about this ?

[DonggeLiu]

Would rust suit you ?

I could not think of any reasons against Rust or the candidate you proposed, but let's wait for @oliverchang in case I missed anything.

check that the questions field of this structure is equal to 1

IIUC, this questions field in the header represents the QDCOUNT (i.e., the number of entries in the question section), which I guess could be more than one in general?

[catenacyber]

The reason against Rust is introducing a new language/toolchain for SystemSan (and the maintenance cost/complexity that comes with it)

IIUC, this questions field in the header represents the QDCOUNT (i.e., the number of entries in the question section), which I guess could be more than one in general?

Indeed, but I do not know where/when/how my software will issue such a request with multiple domains... Would you have an example ?

[catenacyber]

ping @alan32liu what should be done for this PR ? Try it as is ? Or change something ?

[DonggeLiu]

Let's try it as is, if @oliverchang does not have any other suggestions?

[oliverchang]

@jonathanmetzman can help merge and evaluate this. Sorry for the delay on this.