| Message ID | 20221014174342.3216982-1-rasmus.villemoes@prevas.dk |
|---|---|
| Headers | show |
| Series | broken CVE fix (b85d130ea0ca) | expand |
Hi Rasmus, On Fri, Oct 14, 2022 at 2:44 PM Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > tl;dr: b85d130ea0ca didn't fix the CVE(s), but did break tftp of > certain file sizes - which is somewhat lucky, since that's how I > noticed in the first place. > > What I at first hoped would be a one-liner trivial fix turned out to > be much more complicated and led me down a rabbit hole of related > fixes. And this isn't even complete, I'm afraid. Details in 3/6. > > 1 and 4 are independent of all the others. 5 is a trivial preparation > for 6; otherwise those are also independent of the others. Finally, 2 > and 3 are my attempts at actually fixing CVE-2022-{30790,30552}, with > 2 essentially lifting the "ensure the payload has non-negative size" > to the first place we can check that instead of relying on that check > to happen in several places. Thanks for the fix: Reviewed-by: Fabio Estevam <festevam@denx.de>
On 14/10/2022 19.43, Rasmus Villemoes wrote: > tl;dr: b85d130ea0ca didn't fix the CVE(s), but did break tftp of > certain file sizes - which is somewhat lucky, since that's how I > noticed in the first place. > At this point it seems unlikely that any more comments or reviews will come, so perhaps its time to get these (all 7) merged to master, so that they will get some wider testing before the January release? Rasmus
On Mon, Nov 14, 2022 at 10:35:51AM +0100, Rasmus Villemoes wrote: > On 14/10/2022 19.43, Rasmus Villemoes wrote: > > tl;dr: b85d130ea0ca didn't fix the CVE(s), but did break tftp of > > certain file sizes - which is somewhat lucky, since that's how I > > noticed in the first place. > > > > At this point it seems unlikely that any more comments or reviews will > come, so perhaps its time to get these (all 7) merged to master, so that > they will get some wider testing before the January release? Yes, I'd like to see a net PR with this and perhaps a few other mature things?
On Mon, Nov 14, 2022 at 10:04 AM Tom Rini <trini@konsulko.com> wrote: > > On Mon, Nov 14, 2022 at 10:35:51AM +0100, Rasmus Villemoes wrote: > > On 14/10/2022 19.43, Rasmus Villemoes wrote: > > > tl;dr: b85d130ea0ca didn't fix the CVE(s), but did break tftp of > > > certain file sizes - which is somewhat lucky, since that's how I > > > noticed in the first place. > > > > > > > At this point it seems unlikely that any more comments or reviews will > > come, so perhaps its time to get these (all 7) merged to master, so that > > they will get some wider testing before the January release? > > Yes, I'd like to see a net PR with this and perhaps a few other mature > things? Ramon, Joe?
On 17/11/2022 01.32, Fabio Estevam wrote: > On Mon, Nov 14, 2022 at 10:04 AM Tom Rini <trini@konsulko.com> wrote: >> >> On Mon, Nov 14, 2022 at 10:35:51AM +0100, Rasmus Villemoes wrote: >>> On 14/10/2022 19.43, Rasmus Villemoes wrote: >>>> tl;dr: b85d130ea0ca didn't fix the CVE(s), but did break tftp of >>>> certain file sizes - which is somewhat lucky, since that's how I >>>> noticed in the first place. >>>> >>> >>> At this point it seems unlikely that any more comments or reviews will >>> come, so perhaps its time to get these (all 7) merged to master, so that >>> they will get some wider testing before the January release? >> >> Yes, I'd like to see a net PR with this and perhaps a few other mature >> things? > > Ramon, Joe? Ping. If those two CVEs and the tftp brokenness are to get fixed in 2023.01, now is the time to pull in these patches, or provide a viable alternative.
tl;dr: b85d130ea0ca didn't fix the CVE(s), but did break tftp of certain file sizes - which is somewhat lucky, since that's how I noticed in the first place. What I at first hoped would be a one-liner trivial fix turned out to be much more complicated and led me down a rabbit hole of related fixes. And this isn't even complete, I'm afraid. Details in 3/6. 1 and 4 are independent of all the others. 5 is a trivial preparation for 6; otherwise those are also independent of the others. Finally, 2 and 3 are my attempts at actually fixing CVE-2022-{30790,30552}, with 2 essentially lifting the "ensure the payload has non-negative size" to the first place we can check that instead of relying on that check to happen in several places. Rasmus Villemoes (6): net: improve check for no IP options net: compare received length to sizeof(ip_hdr), not sizeof(ip_udp_hdr) net: (actually/better) deal with CVE-2022-{30790,30552} net: fix ip_len in reassembled IP datagram net: tftp: use IS_ENABLED(CONFIG_NET_TFTP_VARS) instead of #if net: tftp: sanitize tftp block size, especially for TX net/net.c | 24 +++++++++---- net/tftp.c | 102 ++++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 92 insertions(+), 34 deletions(-)