Attention is currently required from: Andrey Petrov, Jérémy Compostella, Martin L Roth, Maximilian Brune, Nico Huber, Ronak Kanabar.
2 comments:
File src/commonlib/include/commonlib/region.h:
Patch Set #1, Line 106: offset > SIZE_MAX || size > SIZE_MAX
size_t/SIZE_MAX are implementation defined. I'm rather sure that compiled […]
I agree with Max, I don't really see the point of this (and changing all the types to `unsigned long long`). Are you worried about someone calling `region_create()` with a literal `0x100000000ULL`? That seems unrealistic in practice to me, I can't think of a scenario where that would happen. And in all examples where regions are created from values that are passed in as parameters from elsewhere, those parameters ought to already be `size_t`s.
Also, `SIZE_MAX` may be different in cbfstool than in coreboot, so if this is supposed to protect against larger values from cbfstool getting encoded somewhere that coreboot later interprets as a region, it wouldn't do that either.
File util/cbfstool/cbfstool.c:
Patch Set #1, Line 334: if (region_create_untrusted(
These regions can come from the command line. It's a minor difference with […]
They're parsed via `strtol()` so they already cannot overflow a `size_t`. I think we should do all argument validation at the moment of parsing and then we don't need to worry about it later.
To view, visit change 79905. To unsubscribe, or for help writing mail filters, visit settings.