Page MenuHomeFreeBSD

bspatch: add sanity checks on sizes
ClosedPublic

Authored by emaste on Aug 23 2016, 11:26 PM.
Tags
None
Referenced Files
F137184668: D7619.id19599.diff
Fri, Nov 21, 10:10 AM
F137183687: D7619.id19679.diff
Fri, Nov 21, 10:05 AM
F137183336: D7619.id20101.diff
Fri, Nov 21, 10:03 AM
F137183290: D7619.id.diff
Fri, Nov 21, 10:03 AM
F137183018: D7619.diff
Fri, Nov 21, 10:01 AM
Unknown Object (File)
Mon, Nov 17, 8:27 AM
Unknown Object (File)
Sun, Nov 9, 7:41 AM
Unknown Object (File)
Fri, Oct 24, 1:39 PM
Subscribers

Details

Summary

Note that on i386 this introduces a new 2GB limit, but this is not a concern in practice. Also avoid allocating an extra byte in the old and new file content buffers.

Based on the "NON-CRYPTANALYTIC ATTACKS AGAINST FREEBSD UPDATE COMPONENTS" gist.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

emaste retitled this revision from to bspatch: add sanity checks on sizes.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added reviewers: allanjude, delphij, glebius.
emaste added a subscriber: cperciva.
usr.bin/bsdiff/bspatch/bspatch.c
76–77

On i386 ssize_t is 4 bytes, so would truncate from offtin / lseek.

123

SSIZE_MAX introduces a 2G limit, but we already had an implicit 2G limit for proper operation based on the previous newsize type.

155

Same 2G argument as above.

allanjude edited edge metadata.
This revision is now accepted and ready to land.Aug 25 2016, 4:57 PM
cem added a reviewer: cem.
cem added a subscriber: cem.

Changes seem fine to me. This code is really ugly, though.

usr.bin/bsdiff/bspatch/bspatch.c
123

The gist checks for newsize <= 0. Is zero a valid newsize?

usr.bin/bsdiff/bspatch/bspatch.c
123

Yes, a zero byte output file is legitimate.

kib added a reviewer: kib.
This revision was automatically updated to reflect the committed changes.