Page MenuHomeFreeBSD

dummynet: Fix socket option length validation for IP_DUMMYNET3
ClosedPublic

Authored by markj on Nov 26 2021, 4:02 PM.
Tags
None
Referenced Files
F134714304: D33133.id99171.diff
Tue, Nov 4, 3:34 AM
F134697112: D33133.id99057.diff
Tue, Nov 4, 1:20 AM
F134696724: D33133.id99171.diff
Tue, Nov 4, 1:17 AM
F134696683: D33133.id99057.diff
Tue, Nov 4, 1:17 AM
F134696661: D33133.id99152.diff
Tue, Nov 4, 1:17 AM
F134696399: D33133.id.diff
Tue, Nov 4, 1:16 AM
F134696381: D33133.id99152.diff
Tue, Nov 4, 1:16 AM
F134696280: D33133.diff
Tue, Nov 4, 1:15 AM

Details

Summary

The socket option handler tries to ensure that the option length is no
larger than some reasonable maximum, and no smaller than sizeof(struct
dn_id). But the loaded option length is stored in an int, which is
converted to an unsigned integer for the comparison with a size_t, so
negative values are not caught and get passed to malloc().

Use a cast to enforce the intended comparison. Found by code
inspection.

MFC after: 1 week
Sponsored by: The FreeBSD Foundation

Diff Detail

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

Event Timeline

Wouldn't it make more sense to change l to be a size_t? sopt->sopt_valsize is a size_t, and the arguments to sootcopyin() are also size_t.
It's only do_config() which takes an int, and that may want to be changed to take a size_t as well.

In D33133#748895, @kp wrote:

Wouldn't it make more sense to change l to be a size_t? sopt->sopt_valsize is a size_t, and the arguments to sootcopyin() are also size_t.
It's only do_config() which takes an int, and that may want to be changed to take a size_t as well.

Yeah, that's the right solution.

Make do_config() take a size_t instead.

This revision is now accepted and ready to land.Nov 29 2021, 5:12 PM