Re: [PATCH] Remove HFS support

[Daniel Axtens <dja@axtens.net>]

"Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> writes:

> Le ven. 26 août 2022, 15:47, Daniel Axtens <dja@axtens.net> a écrit :
>
>> Let me answer this out of order.
>>
>> > I understand the need to sometimes get rid of old code, but since the HFS
>> > module can be blacklisted as Vladimir explains, I don't really understand
>> > the reasoning in this particular case.
>>
>> I want _all_ grub code to reach a minimum standard of not crashing or
>> corrupting memory in the presence of malicious input. HFS does not reach
>> that standard.
>>
> That is a very high standard. Products with a huge security team like
> Chrome don't reach this standard. It's reasonable that you submit the
> improvements. Also it's reasonable for you to blacklist code that gets in
> the way of security. E.g. all compressors that are not used should be
> blacklisted.

ext and fat file systems (and several other more obsure file systems)
and all our image parsers reach this standard, best as I can tell. As
far as I can tell the grub IPv4 networking stack does too, although I am
not as certain that my coverage was very thorough.

Several of us are actively working to get all of grub to this
standard. grub is a lot simpler than Chrome, so I am optimistic.

>> If you or someone else (someone from Gentoo, perhaps?) want make it fuzz
>> clean, then that'd be great. If no-one is able to bring it up to what is
>> *not* an especially high standard, then it should be considered
>> abandoned by developers and therefore removed.
>>
> Show me the fuzzes that create problems and I'll improve the code

The following two files cause crashes on stock grub-fstest

stack overflow (unbounded recursion): files.intermittent.network/grub/hfs.stack-overflow
stack buffer overflow -> eventual segv: files.intermittent.network/grub/hfs.stack-buffer-overflow

There are an additional set of files that cause crashes when grub is
compiled with ASAN:

files.intermittent.network/grub/hfs.tar.xz (18MB, 210MB uncompressed)

There are 222 files. The corpus is not de-duplicated (there are not
222 unique bugs) and includes the two files called out above, plus
other some different heap buffer overflows.

I compile grub with ASAN using:
ASAN_OPTIONS=detect_leaks=0 make CFLAGS="-fsanitize=address" -j8

Modern gcc works fine. grub-emu will fail to link, but grub-fstest
should build fine.

In all cases, the crashes reproduce with:

./grub-fstest <file> ls '(loop0)/'

Good luck, the stack-overflow one in particular looks especially
painful.

I will leave your other points for others to address. 

Kind regards,
Daniel