Fix unchecked offsets during entry read

[Ant1-Provot]

Description

Hi, working on camlzip I identified a bug that makes the read_entry function goes into an infinite loop for entries that have a negative offset or an offset bigger than the size of the file.

In that very specific case, the recursive function uncompr_finish L116 goes infinite as inflate always returns finished = false ; _ leading to an infinite loop.

It is hard to discuss the likeliness of this bug, as it's very unlikely to happen naturally in the wild, but is however very easy to craft with a very limited knowledge of the pkzip file format or application fuzzing.

For the record, zip obviously doesn't appreciate such crafted files, but doesn't go infinite :

🦆 ~/workspace $ unzip demo.zip                                                                            
Archive:  demo.zip
warning:  filename too long--truncating.
[ non-empty.txtUT^I ]
non-empty.txtUT^I:  bad extra field length (local)

Reproduction step

I created a little file that reproduces the bug minimally.

It contains a single entry of a non empty file, containing some text. Then, I tweaked both Filename Length (0x001A) and Extra Length (0x001C) with the maximal values (0xffff).

🦆 ~/workspace$ zipdetails demo.zip                                                                     

0000 LOCAL HEADER #1       04034B50
0004 Extract Zip Spec      14 '2.0'
0005 Extract OS            00 'MS-DOS'
0006 General Purpose Flag  0000
     [Bits 1-2]            0 'Normal Compression'
0008 Compression Method    0008 'Deflated'
000A Last Mod Time         59146C49 'Tue Aug 20 13:34:18 2024'
000E CRC                   8E94FE83
0012 Compressed Length     00000110
0016 Uncompressed Length   000001C3
001A Filename Length       FFFF
001C Extra Length          FFFF
Truncated file (got 418, wanted 65535):

You can find attached this file here : demo.zip

Type of change

This PR is a bug fix.

The implementation was straightforward, added a specific check in goto_entry raising an error in the case it happens.

Style of change

I tried to stick as much as possible to the coding style, but I have to admit I didn't find the coding convention of the repo, nor any formatting enforcement tool, so if I did wrong please tell me ! 😄

How Has This Been Tested?

• [x] Ensured the issue happened on the attached file before my fix
• [x] Ensured the issue did not happen on the attached file after my fix.

If you believe adding a test over that would make sense I'll also do it with pleasure. I believe having the attached file as part of the codebase would be a good call, but I'd like your opinion first and the way you believe it would be best to implement it.

Additional conversation

I noticed that the current codebase has limited test coverage, particularly around corner cases and unit tests. Having more tests could help ensure the reliability and maintainability of the code, especially as camlzip is a central element of the ocaml ecosystem.

If you're interested, I'd be happy to help write some tests or collaborate on improving the test coverage. Let me know how I can assist! 🥳