[pe] Fixed bug found by fuzzing.

[haberman]

With bad section headers, the PE parser could report a range that exceeded the file bounds.

cc @learn-more

[learn-more]

@haberman I would be surprised if there were no bugs like this ;) The only validation done currently is making sure bloaty is not reading out of bounds, there are hardly any validations done on the validity of the PE file itself.

There are 2 reasons for this:

• The code is still in very early development, littering it with all kind of checks to make it 'nicer' would probably not help speeding it up
• There are two different approaches which could be taken:
  • Validating data 'as good as possible', largely ignoring the kind of PE files accepted, mainly focusing on what would theoretically be possible inside the bounds of the format
  • Validating data 'as close to how PE files are accepted', meaning that we would have to investigate what kind of weirdness is allowed, and what not. (A good starting point would be https://corkamiwiki.github.io/PE )

[haberman]

@learn-more that totally makes sense. This bug only crossed my radar because, with the right (malformed) input, it could trigger an assert failure in bloaty.cc. The main part of Bloaty verifies that the "file size" total is the same as the actual file size.

The fuzzer found a 73-byte input that could yield the following output:

    FILE SIZE        VM SIZE                                                                                                                                                                                                                                                                
 --------------  --------------                            
 100.0%   514Mi 100.0%   527Mi                                                                                                                                                                                                                                                              
   0.0%      73   0.0%      73    [PE Headers]                         
 100.0%   514Mi 100.0%   528Mi    TOTAL

This triggered an assert failure in Bloaty, since 514Mi != 73 bytes. This is totally understandable, all of the other file formats had bugs like these at first.

My thoughts on how Bloaty should handle malformed data are evolving. I'm leaning towards the following model:

• Bloaty should do its best to gracefully handle unexpected input.
• Bloaty should never bail out completely unless it's impossible to recover.

This PR goes against the second goal for now; I'm keeping PE consistent with what the other file formats do. At some point, probably after ABSL releases logging support, I want to revisit this and log a warning instead of throwing an exception).

[haberman]

https://corkamiwiki.github.io/PE

btw, this is a very interesting link, thanks for sharing.