SystemSan: only report arbitrary file open for writes

[catenacyber]

File opens for read only seem so far to be :

• config includers (like yaml fuzzers)
• files parsed under a specific format, which makes it unlikely that an attacker can abuse it to leak some secret.

[catenacyber]

cc @oliverchang @alan32liu

[evverx]

As discussed on the elfutils mailing list I think it's an interesting idea but I'm not sure that just flagging open is good enough because those suspicious attempts to open files turn into actual bugs only if they are followed by something that actually can expose sensitive data, overwrite random files and so on and to catch that the sanitizer should be able to keep track of data flow among other things. Without that it mostly flags code smells that should probably be investigated on a case-by-case basis. If OSS-Fuzz is planning to report "maybe bugs" I think there should be a way to mark some bugs as false positives that should be ignored by the sanitizer on a case-by-case basis.

[catenacyber]

@evverx this PR restricts the open syscalls to the ones which overwrite random files ;-)

It is planned to add some configuration option cf #8497 which will likely be per target (because something like curl -X PUT file://localhost/random is indeed a feature to overwrite random files)

[evverx]

It is planned to add some configuration option cf #8497 which will likely be per target

As far as I understand the idea is to make it possible to turn off SystemSan either fully or for example keep the part catching code execution on and turn off the part flagging open. I think it makes sense but I can imagine scenarios where it can be desirable to keep catching arbitrary opens even though sometimes it produces false positives and for that to work I think the sanitizer should be somehow aware of codepaths/backtraces leading to false positives so that it could ignore them and keep going instead of crashing.

Regarding O_RDONLY I suspect it produces a lot of false positives so it should probably be turned off by default but if say projects provide code that can be run by suid binaries I think OSS-Fuzz should allow them to bring O_RDONLY back to make it easier to find places like that and make sure that whatever it is they parse isn't included in error messages for example.

[evverx]

I think what's interesting about this sanitizer is that it can produce bug reports that can be considered false positives by some projects for various reasons and can be considered bugs at the same time (python/cpython#45385 comes to mind). I'm not sure but it would probably make sense to keep finding and reporting issues like that regardless of whether projects choose to not treat them as bugs.

Anyway it could be that I'm overthinking this.

[catenacyber]

@jonathanmetzman would it be possible for SystemSan config to have some option like detect_file_open that could be either all, none, or write where the latest would be the default ?

[catenacyber]

@alan32liu should we merge this for now ?

[oliverchang]

Are there any examples of legitimate bugs from the current set? If not, it may be better to leave this entire sanitizer as an configurable option wrt read/write and make it off by default.

[catenacyber]

The legitimate bug found so far was unrar CVE-2022-30333 which was a write.

I am ok with the config option. But in the meantime (as long as config options for SystemSan do not exist), we can restrict it to writes...

[jonathanmetzman]

@jonathanmetzman would it be possible for SystemSan config to have some option like detect_file_open that could be either all, none, or write where the latest would be the default ?

Yeah I think we can do this.

[oliverchang]

Note I created #9142 to completely disable this for now, since right now it's generating a lot of noise and potentially also blocking us from finding things with our other sanitizers.

We can bring it back once we have flag support.

[catenacyber]

I think this was a better alternative than #9142 as the noise is in the reads, not in the writes...

Should I close this then ?

[catenacyber]

Any updates on SystemSanitizer ?

I still think detecting arbitrary file write is better than nothing or detecting arbitrary file open (which causes noise because of arbitrary file open for reads)