Page MenuHomeFreeBSD

hastd: Fix nv data size check
ClosedPublic

Authored by des on Jul 29 2025, 6:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 10, 5:26 AM
Unknown Object (File)
Fri, Oct 10, 5:26 AM
Unknown Object (File)
Fri, Oct 10, 5:26 AM
Unknown Object (File)
Fri, Oct 10, 5:26 AM
Unknown Object (File)
Fri, Oct 10, 5:26 AM
Unknown Object (File)
Thu, Oct 9, 11:57 PM
Unknown Object (File)
Mon, Sep 22, 3:02 AM
Unknown Object (File)
Sat, Sep 20, 5:39 PM
Subscribers

Details

Summary

The data size check, as currently written, can be defeated by providing
a very large number that rounds up to 0, which will pass the check
(because zero plus the size of the header and name is smaller than the
size of the message) but cause a segfault later when used to index the
data array.

Rewrite the data size check to take rounding into account, and add a
cast to ensure the name size can't round up to zero.

MFC after: 1 week
PR: 266827

Diff Detail

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

Event Timeline

des requested review of this revision.Jul 29 2025, 6:01 PM
markj added inline comments.
sbin/hastd/nv.c
250–251

Isn't it still possible to have roundup2(dsize, 8) < size - NVH_HSIZE(nvh) even if the newly added check passes? Will that cause problems when ptr and size are adjusted at the end of the loop?

sbin/hastd/nv.c
250–251

You're right, if dsize is smaller than size - NVH_SIZE(nvh) but rounds up to something larger, then size ends up wrapping around.

des marked an inline comment as done.Aug 4 2025, 5:29 PM

I think this patch fixes the problem at hand, but see the inline comment.

sbin/hastd/nv.c
231

NVH_HSIZE() also has a problem if roundup2(nvh->nvh_namesize, 8) == 0, I think.

This revision is now accepted and ready to land.Aug 4 2025, 5:37 PM
sbin/hastd/nv.c
231

I'm tempted to just put a hard limit of INT_MAX on both nvh_namesize and nvh_dsize...

231

that won't be enough on 32-bit platforms though :(

des marked an inline comment as done.
This revision now requires review to proceed.Aug 5 2025, 9:20 AM
This revision is now accepted and ready to land.Aug 5 2025, 12:50 PM
This revision was automatically updated to reflect the committed changes.