Comment 6 for bug 1899193

Revision history for this message
Julian Andres Klode (juliank) wrote :

#1 and #2 only require fixes in apt:

So #2. Is it really infinite, isn't it basically too big? Similar to when you start reading the file, but the file is truncated.

Now, apparently we do require that we know the size of the arfile (which is stored in Left), so what we should do I guess is

if (Memb->Size > Left) {
    delete Memb;
    return _error->Error(_("Invalid archive member header"))
}

which fixes the issue and also detects truncated files. Well, I guess the message could also say the archive is truncated, but everything else uses this message.

Now #1 we know that Memb->Size is as correct as it will get, so the step here seems to be to add

if (Len > Memb->Size) {
    delete Memb;
    return _error->Error(_("Invalid archive member header"))
}

This ensures that the length we parse for the long name is shorter than the member, which is correct.

#3 needs wrapping object in a smart pointer so the fd is relased. This also needs similar changes in ararchive_new, and possibly a whole bunch of other code that opens files on its own. Further investigation is necessary.

The good thing is that we should be able to get unit tests for all of those. For FDs we count open fds before and after an erronous call, and #1#2 we can check that the malicious debs fail with an error :)