[PATCH opkg] libopkg: move file size check after checksum verification

Baptiste Jonglez baptiste at bitsofnetworks.org
Mon Aug 24 09:04:15 EDT 2020


On 24-08-20, Henrique de Moraes Holschuh wrote:
> On 24/08/2020 09:01, Baptiste Jonglez wrote:
> > On 24-08-20, Henrique de Moraes Holschuh wrote:
> > > On 24/08/2020 07:53, Baptiste Jonglez wrote:
> > > > It is more user-friendly to tell the user that the checksum is wrong, so
> > > > move the file size check at the end.
> > > 
> > > It is also far more expensive in the failure case, not to mention the fact
> > > that you're going to process data you KNOW to be wrong when you could have
> > > easily avoided it.
> > 
> > I agree, this leads to unnecessary processing in the failure case,
> > i.e. when the size & checksum are wrong.
> > 
> > However, this failure case is rather unexpected, and I doubt that a
> > slightly higher processing time (if it is even measurable) is an issue
> > when you are dealing with corrupted packages.
> 
> Give it an ultra-large sparse file as input, on purpose (attacker) or due to
> corrupted filesystem with weird inode data.  Suddenly, you would be much
> happier had you checked the file size first (i.e. without this change)...

Ok, I can see how it could go wrong.

Note that the size check is quite recent (January 2019).  Before this size
check was introduced, the situation you describe could already happen.

> IMHO, your proposed change should be backed by a strong reason that offsets
> the increased risk.  Again IMHO, the commit log doesn't currently provide a
> strong enough reason.
> 
> But that's IMO.  Let's wait for other opinions (or you could provide a
> stronger reason to apply this change, perhaps?)

No, it's really just cosmetic, the change was based on a private discussion.

In the end I don't mind one way of the other, and your argument makes
sense regarding the trade-off.  Please disregard this patch, and I will
send a v2 of the other patch so that it doesn't depend on this one.

Baptiste
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20200824/55c6b469/attachment.sig>


More information about the openwrt-devel mailing list