Page MenuHomeFreeBSD

diff: Fix integer overflow.
ClosedPublic

Authored by des on Jul 27 2024, 5:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jun 5, 11:15 PM
Unknown Object (File)
Fri, May 30, 1:26 AM
Unknown Object (File)
Mon, May 26, 10:00 PM
Unknown Object (File)
Wed, May 21, 10:59 AM
Unknown Object (File)
Mon, May 19, 3:30 AM
Unknown Object (File)
Mon, May 12, 8:24 AM
Unknown Object (File)
Sun, May 11, 8:24 AM
Unknown Object (File)
Apr 30 2025, 11:54 PM
Subscribers

Details

Summary

The legacy Stone algorithm uses int to represent line numbers, array
indices, and array lengths. If given inputs approaching INT_MAX lines,
it would overflow and attempt to allocate ridiculously large amounts of
memory. To avoid this without penalizing non-pathological inputs,
switch a few variables to size_t and add checks while and immediately
after reading both inputs.

MFC after: 3 days
PR: 280371
Sponsored by: Klara, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

des requested review of this revision.Jul 27 2024, 5:42 PM
allanjude added a subscriber: allanjude.
allanjude added inline comments.
usr.bin/diff/diffreg.c
558

Was it intentional to drop the RH_EOF case here?

This revision is now accepted and ready to land.Jul 27 2024, 6:00 PM
des marked an inline comment as done.Jul 27 2024, 6:03 PM
des added inline comments.
usr.bin/diff/diffreg.c
558

Yes, its only purpose was to silence a compiler warning.

des marked an inline comment as done.Jul 27 2024, 6:04 PM
yuri requested changes to this revision.Jul 27 2024, 6:57 PM
yuri added a subscriber: yuri.

Shouldn't

if (len[0] > INT_MAX - 2)

be

if (len[0] > SIZE_MAX - 2)

?

And shouldn't all int types representing file size be made 64-bit?
They should be uint64_t.

This revision now requires changes to proceed.Jul 27 2024, 6:57 PM

Shouldn't

if (len[0] > INT_MAX - 2)

be

if (len[0] > SIZE_MAX - 2)

?

No, because we need len[0] + 2 to not exceed INT_MAX.

And shouldn't all int types representing file size be made 64-bit?
They should be uint64_t.

The variables used to store file sizes are already size_t. The algorithm operates on line numbers, not byte offsets, and it uses negative values. I'm OK with refusing to handle files with more than 2^31 lines.

This revision is now accepted and ready to land.Jul 29 2024, 1:59 PM
This revision was automatically updated to reflect the committed changes.