Vulnerability Disclosure in net/

Vulnerability Disclosure in net/

[Nicolas Bidron]

Hello,

We found a couple of bugs in net/net.s in the IP defragmentation 
function __net_defragment(). Below the writeup for the 2 bugs:

-----------BUG 1-----------

# Hole Descriptor Overwrite in U-Boot IP Packet Defragmentation Leads to 
Arbitrary Out of Bounds Write Primitive (CVE-TBD)

|  |  |
| --- | --- |
|Project | U-Boot |
|Project URL | https://github.com/u-boot/u-boot |
|Versions affected | all versions up to commit TBD |
|Systems affected | All systems defining `CONFIG_IP_DEFRAG` |
|CVE identifier | TBD |
|Advisory URL | TBD |
|Risk | Critical 9.6 (CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H) |
|Authors | Nicolas Guigo, Nicolas Bidron |

### Summary

U-boot is a popular boot loader for embedded systems with 
implementations for a large number of architectures and prominent in 
most linux based embedded systems.

### Location

In `u-boot/net/net.c` the `__net_defragment` function line 900 through 1018.

### Impact

The U-Boot implementation of 
[RFC815](https://datatracker.ietf.org/doc/html/rfc815) IP DATAGRAM 
REASSEMBLY ALGORITHMS is susceptible to a Hole Descriptor overwrite 
attack which ultimately leads to an arbitrary write primitve.

### Description

In compiled versions of U-Boot that define CONFIG_IP_DEFRAG, a value of 
`ip->ip_len` (IP packet header's Total Length) higher than `IP_HDR_SIZE` 
and strictly lower than `IP_HDR_SIZE+8` will lead to a value for `len` 
comprised between `0` and `7`. This will ultimately result in a 
truncated division by `8` resulting value of `0` forcing the hole 
metadata and fragment to point to the same location. The subsequent 
memcopy will overwrite the hole metadata with the fragment data. Through 
a second fragment, this can be exploited to write to an arbitrary offset 
controlled by that overwritten hole metadata value.

This bug is only exploitable locally as it requires crafting two packets 
the first of which would most likely be dropped through routing due to 
its unexpectedly low Total Length. However, this bug can potentially be 
exploited to root linux based embedded devices locally.

```C
static struct ip_udp_hdr *__net_defragment(struct ip_udp_hdr *ip, int *lenp)
{
     static uchar pkt_buff[IP_PKTSIZE] __aligned(PKTALIGN);
     static u16 first_hole, total_len;
     struct hole *payload, *thisfrag, *h, *newh;
     struct ip_udp_hdr *localip = (struct ip_udp_hdr *)pkt_buff;
     uchar *indata = (uchar *)ip;
     int offset8, start, len, done = 0;
     u16 ip_off = ntohs(ip->ip_off);

     /* payload starts after IP header, this fragment is in there */
     payload = (struct hole *)(pkt_buff + IP_HDR_SIZE);
     offset8 =  (ip_off & IP_OFFS);
     thisfrag = payload + offset8;
     start = offset8 * 8;
     len = ntohs(ip->ip_len) - IP_HDR_SIZE;
```

The last line of the previous excerpt from `u-boot/net/net.c` shows how 
the attacker can control the value of `len` to be strictly lower than 
`8` by issuing a packet with `ip_len` between `21` and `27` 
(`IP_HDR_SIZE` has a value of `20`).

Also note that `offset8` here is `0` which leads to `thisfrag = payload`.

```C
     } else if (h >= thisfrag) {
         /* overlaps with initial part of the hole: move this hole */
         newh = thisfrag + (len / 8);
         *newh = *h;
         h = newh;
         if (h->next_hole)
             payload[h->next_hole].prev_hole = (h - payload);
         if (h->prev_hole)
             payload[h->prev_hole].next_hole = (h - payload);
         else
             first_hole = (h - payload);

     } else {
```

Lower down the same function, execution reaches the above code path. 
Here, `len / 8` evaluates to `0` leading to `newh = thisfrag`. Also note 
that `first_hole` here is `0` since `h` and `payload` point to the same 
location.

```C
     /* finally copy this fragment and possibly return whole packet */
     memcpy((uchar *)thisfrag, indata + IP_HDR_SIZE, len);
```

Finally, in the above excerpt the `memcpy` overwrites the hole metadata 
since `thisfrag` and `h` both point to the same location. The hole 
metadata is effectively overwritten with arbitrary data from the 
fragmented IP packet data. If `len` was crafted to be `6`, `last_byte`, 
`next_hole`, and `prev_hole` of the `first_hole` can be controlled by 
the attacker.

Finally the arbitrary offset write occurs through a second fragment that 
only needs to be crafted to write data in the hole pointed to by the 
previously controlled hole metadata (`next_hole`) from the first packet.

### Recommendation

Handle cases where `len` is strictly lower than 8 by preventing the 
overwrite of the hole metadata during the memcpy of the fragment. This 
could be achieved by either:
* Moving the location where the hole metadata is stored when `len` is 
lower than `8`.
* Or outright rejecting fragmented IP datagram with a Total Length 
(`ip_len`) lower than 28 bytes which is the minimum valid fragmented IP 
datagram size (as defined as the minimum fragment of 8 octets in the IP 
Specification Document: 
[RFC791](https://datatracker.ietf.org/doc/html/rfc791) page 25).

----------BUG 2----------

# Large buffer overflow leads to DoS in U-Boot IP Packet Defragmentation 
Code (CVE-TBD)

|  |  |
| --- | --- |
|Project | U-Boot |
|Project URL | https://github.com/u-boot/u-boot |
|Versions affected | all versions up to commit TBD |
|Systems affected | All systems defining `CONFIG_IP_DEFRAG` |
|CVE identifier | TBD |
|Advisory URL | TBD |
|Risk | High 7.1 (CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:N/I:L/A:H) |
|Authors | Nicolas Guigo, Nicolas Bidron |

### Summary

U-boot is a popular boot loader for embedded systems with 
implementations for a large number of architectures and prominent in 
most linux based embedded systems.

### Location

`u-boot/net/net.c` lines 915 and 1011.

### Impact

The U-Boot implementation of 
[RFC815](https://datatracker.ietf.org/doc/html/rfc815) IP DATAGRAM 
REASSEMBLY ALGORITHMS is susceptible to a buffer overflow through a 
specially crafted fragmented IP Datagram with an invalid Total Length 
which creates Denial of Service conditions.

### Description

In compiled versions of U-Boot that define CONFIG_IP_DEFRAG, a value of 
`ip->ip_len` (IP packet header's Total Length) lower than `IP_HDR_SIZE` 
will lead to a negative value for `len` which will ultimately result in 
a buffer overflow during the subsequent `memcpy` that uses `len` as it's 
`count` parameter.

This bug is only exploitable on local ethernet as it requires crafting 
an invalid packet to include an unexpected `ip_len` value in the IP UDP 
header that's lower than the minimum accepted Total Length of a packet 
(21 as defined in the IP Specification Document: 
[RFC791](https://datatracker.ietf.org/doc/html/rfc791)). Such packet 
would in all likelihood be dropped while being routed to its final 
destination through most routing equipment and as such requires the 
attacker to be in a local position in order to be exploited.

```C
static struct ip_udp_hdr *__net_defragment(struct ip_udp_hdr *ip, int *lenp)
{
     static uchar pkt_buff[IP_PKTSIZE] __aligned(PKTALIGN);
     static u16 first_hole, total_len;
     struct hole *payload, *thisfrag, *h, *newh;
     struct ip_udp_hdr *localip = (struct ip_udp_hdr *)pkt_buff;
     uchar *indata = (uchar *)ip;
     int offset8, start, len, done = 0;
     u16 ip_off = ntohs(ip->ip_off);

     /* payload starts after IP header, this fragment is in there */
     payload = (struct hole *)(pkt_buff + IP_HDR_SIZE);
     offset8 =  (ip_off & IP_OFFS);
     thisfrag = payload + offset8;
     start = offset8 * 8;
     len = ntohs(ip->ip_len) - IP_HDR_SIZE;
```

The last line of the previous excerpt from `u-boot/net/net.c` shows 
where the undeflow to a negative `len` value occurs if `ip_len` is set 
to a value strictly lower than 20 (`IP_HDR_SIZE` being 20). Also note 
that in the above excerpt the `pkt_buff` buffer has a size of 
`CONFIG_NET_MAXDEFRAG` which defaults to 16 KB but can range from 1KB to 
64 KB depending on configurations.

```C
     /* finally copy this fragment and possibly return whole packet */
     memcpy((uchar *)thisfrag, indata + IP_HDR_SIZE, len);
```

In the above excerpt the `memcpy` overflows the destination by 
attempting to make a copy of nearly 4 gigabytes in a buffer that's 
designed to hold `CONFIG_NET_MAXDEFRAG` bytes at most which leads to a DoS.

### Recommendation

Stop processing of the packet if `ip_len` is lower than 21 (as defined 
by the minimum length of a data carrying datagram in the IP 
Specification Document: 
[RFC791](https://datatracker.ietf.org/doc/html/rfc791) page 34).


Nicolas Bidron, Nicolas Guigo

Re: Vulnerability Disclosure in net/

[Ramon Fried]

On Wed, May 18, 2022 at 7:14 PM Nicolas Bidron
<nicolas.bidron@nccgroup.com> wrote:
>
> Hello,
>
> We found a couple of bugs in net/net.s in the IP defragmentation
> function __net_defragment(). Below the writeup for the 2 bugs:
>
> -----------BUG 1-----------
>
> # Hole Descriptor Overwrite in U-Boot IP Packet Defragmentation Leads to
> Arbitrary Out of Bounds Write Primitive (CVE-TBD)
>
> |  |  |
> | --- | --- |
> |Project | U-Boot |
> |Project URL | https://github.com/u-boot/u-boot |
> |Versions affected | all versions up to commit TBD |
> |Systems affected | All systems defining `CONFIG_IP_DEFRAG` |
> |CVE identifier | TBD |
> |Advisory URL | TBD |
> |Risk | Critical 9.6 (CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H) |
> |Authors | Nicolas Guigo, Nicolas Bidron |
>
> ### Summary
>
> U-boot is a popular boot loader for embedded systems with
> implementations for a large number of architectures and prominent in
> most linux based embedded systems.
>
> ### Location
>
> In `u-boot/net/net.c` the `__net_defragment` function line 900 through 1018.
>
> ### Impact
>
> The U-Boot implementation of
> [RFC815](https://datatracker.ietf.org/doc/html/rfc815) IP DATAGRAM
> REASSEMBLY ALGORITHMS is susceptible to a Hole Descriptor overwrite
> attack which ultimately leads to an arbitrary write primitve.
>
> ### Description
>
> In compiled versions of U-Boot that define CONFIG_IP_DEFRAG, a value of
> `ip->ip_len` (IP packet header's Total Length) higher than `IP_HDR_SIZE`
> and strictly lower than `IP_HDR_SIZE+8` will lead to a value for `len`
> comprised between `0` and `7`. This will ultimately result in a
> truncated division by `8` resulting value of `0` forcing the hole
> metadata and fragment to point to the same location. The subsequent
> memcopy will overwrite the hole metadata with the fragment data. Through
> a second fragment, this can be exploited to write to an arbitrary offset
> controlled by that overwritten hole metadata value.
>
> This bug is only exploitable locally as it requires crafting two packets
> the first of which would most likely be dropped through routing due to
> its unexpectedly low Total Length. However, this bug can potentially be
> exploited to root linux based embedded devices locally.
>
> ```C
> static struct ip_udp_hdr *__net_defragment(struct ip_udp_hdr *ip, int *lenp)
> {
>      static uchar pkt_buff[IP_PKTSIZE] __aligned(PKTALIGN);
>      static u16 first_hole, total_len;
>      struct hole *payload, *thisfrag, *h, *newh;
>      struct ip_udp_hdr *localip = (struct ip_udp_hdr *)pkt_buff;
>      uchar *indata = (uchar *)ip;
>      int offset8, start, len, done = 0;
>      u16 ip_off = ntohs(ip->ip_off);
>
>      /* payload starts after IP header, this fragment is in there */
>      payload = (struct hole *)(pkt_buff + IP_HDR_SIZE);
>      offset8 =  (ip_off & IP_OFFS);
>      thisfrag = payload + offset8;
>      start = offset8 * 8;
>      len = ntohs(ip->ip_len) - IP_HDR_SIZE;
> ```
>
> The last line of the previous excerpt from `u-boot/net/net.c` shows how
> the attacker can control the value of `len` to be strictly lower than
> `8` by issuing a packet with `ip_len` between `21` and `27`
> (`IP_HDR_SIZE` has a value of `20`).
>
> Also note that `offset8` here is `0` which leads to `thisfrag = payload`.
>
> ```C
>      } else if (h >= thisfrag) {
>          /* overlaps with initial part of the hole: move this hole */
>          newh = thisfrag + (len / 8);
>          *newh = *h;
>          h = newh;
>          if (h->next_hole)
>              payload[h->next_hole].prev_hole = (h - payload);
>          if (h->prev_hole)
>              payload[h->prev_hole].next_hole = (h - payload);
>          else
>              first_hole = (h - payload);
>
>      } else {
> ```
>
> Lower down the same function, execution reaches the above code path.
> Here, `len / 8` evaluates to `0` leading to `newh = thisfrag`. Also note
> that `first_hole` here is `0` since `h` and `payload` point to the same
> location.
>
> ```C
>      /* finally copy this fragment and possibly return whole packet */
>      memcpy((uchar *)thisfrag, indata + IP_HDR_SIZE, len);
> ```
>
> Finally, in the above excerpt the `memcpy` overwrites the hole metadata
> since `thisfrag` and `h` both point to the same location. The hole
> metadata is effectively overwritten with arbitrary data from the
> fragmented IP packet data. If `len` was crafted to be `6`, `last_byte`,
> `next_hole`, and `prev_hole` of the `first_hole` can be controlled by
> the attacker.
>
> Finally the arbitrary offset write occurs through a second fragment that
> only needs to be crafted to write data in the hole pointed to by the
> previously controlled hole metadata (`next_hole`) from the first packet.
>
> ### Recommendation
>
> Handle cases where `len` is strictly lower than 8 by preventing the
> overwrite of the hole metadata during the memcpy of the fragment. This
> could be achieved by either:
> * Moving the location where the hole metadata is stored when `len` is
> lower than `8`.
> * Or outright rejecting fragmented IP datagram with a Total Length
> (`ip_len`) lower than 28 bytes which is the minimum valid fragmented IP
> datagram size (as defined as the minimum fragment of 8 octets in the IP
> Specification Document:
> [RFC791](https://datatracker.ietf.org/doc/html/rfc791) page 25).
>
> ----------BUG 2----------
>
> # Large buffer overflow leads to DoS in U-Boot IP Packet Defragmentation
> Code (CVE-TBD)
>
> |  |  |
> | --- | --- |
> |Project | U-Boot |
> |Project URL | https://github.com/u-boot/u-boot |
> |Versions affected | all versions up to commit TBD |
> |Systems affected | All systems defining `CONFIG_IP_DEFRAG` |
> |CVE identifier | TBD |
> |Advisory URL | TBD |
> |Risk | High 7.1 (CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:N/I:L/A:H) |
> |Authors | Nicolas Guigo, Nicolas Bidron |
>
> ### Summary
>
> U-boot is a popular boot loader for embedded systems with
> implementations for a large number of architectures and prominent in
> most linux based embedded systems.
>
> ### Location
>
> `u-boot/net/net.c` lines 915 and 1011.
>
> ### Impact
>
> The U-Boot implementation of
> [RFC815](https://datatracker.ietf.org/doc/html/rfc815) IP DATAGRAM
> REASSEMBLY ALGORITHMS is susceptible to a buffer overflow through a
> specially crafted fragmented IP Datagram with an invalid Total Length
> which creates Denial of Service conditions.
>
> ### Description
>
> In compiled versions of U-Boot that define CONFIG_IP_DEFRAG, a value of
> `ip->ip_len` (IP packet header's Total Length) lower than `IP_HDR_SIZE`
> will lead to a negative value for `len` which will ultimately result in
> a buffer overflow during the subsequent `memcpy` that uses `len` as it's
> `count` parameter.
>
> This bug is only exploitable on local ethernet as it requires crafting
> an invalid packet to include an unexpected `ip_len` value in the IP UDP
> header that's lower than the minimum accepted Total Length of a packet
> (21 as defined in the IP Specification Document:
> [RFC791](https://datatracker.ietf.org/doc/html/rfc791)). Such packet
> would in all likelihood be dropped while being routed to its final
> destination through most routing equipment and as such requires the
> attacker to be in a local position in order to be exploited.
>
> ```C
> static struct ip_udp_hdr *__net_defragment(struct ip_udp_hdr *ip, int *lenp)
> {
>      static uchar pkt_buff[IP_PKTSIZE] __aligned(PKTALIGN);
>      static u16 first_hole, total_len;
>      struct hole *payload, *thisfrag, *h, *newh;
>      struct ip_udp_hdr *localip = (struct ip_udp_hdr *)pkt_buff;
>      uchar *indata = (uchar *)ip;
>      int offset8, start, len, done = 0;
>      u16 ip_off = ntohs(ip->ip_off);
>
>      /* payload starts after IP header, this fragment is in there */
>      payload = (struct hole *)(pkt_buff + IP_HDR_SIZE);
>      offset8 =  (ip_off & IP_OFFS);
>      thisfrag = payload + offset8;
>      start = offset8 * 8;
>      len = ntohs(ip->ip_len) - IP_HDR_SIZE;
> ```
>
> The last line of the previous excerpt from `u-boot/net/net.c` shows
> where the undeflow to a negative `len` value occurs if `ip_len` is set
> to a value strictly lower than 20 (`IP_HDR_SIZE` being 20). Also note
> that in the above excerpt the `pkt_buff` buffer has a size of
> `CONFIG_NET_MAXDEFRAG` which defaults to 16 KB but can range from 1KB to
> 64 KB depending on configurations.
>
> ```C
>      /* finally copy this fragment and possibly return whole packet */
>      memcpy((uchar *)thisfrag, indata + IP_HDR_SIZE, len);
> ```
>
> In the above excerpt the `memcpy` overflows the destination by
> attempting to make a copy of nearly 4 gigabytes in a buffer that's
> designed to hold `CONFIG_NET_MAXDEFRAG` bytes at most which leads to a DoS.
>
> ### Recommendation
>
> Stop processing of the packet if `ip_len` is lower than 21 (as defined
> by the minimum length of a data carrying datagram in the IP
> Specification Document:
> [RFC791](https://datatracker.ietf.org/doc/html/rfc791) page 34).
>
>
> Nicolas Bidron, Nicolas Guigo
>
Hi Nicolas,
Thanks for the research.
I have read your description thoroughly, very interesting.
I will implement fixes to the findings.
Thanks.
Ramon.

Re: Vulnerability Disclosure in net/

[Matthias Brugger]

Hi Ramon,

On 26/05/2022 04:46, Ramon Fried wrote:
> On Wed, May 18, 2022 at 7:14 PM Nicolas Bidron
> <nicolas.bidron@nccgroup.com> wrote:
>>
>> Hello,
>>
>> We found a couple of bugs in net/net.s in the IP defragmentation
>> function __net_defragment(). Below the writeup for the 2 bugs:
>>
>> -----------BUG 1-----------
>>
>> # Hole Descriptor Overwrite in U-Boot IP Packet Defragmentation Leads to
>> Arbitrary Out of Bounds Write Primitive (CVE-TBD)
>>
>> |  |  |
>> | --- | --- |
>> |Project | U-Boot |
>> |Project URL | https://github.com/u-boot/u-boot |
>> |Versions affected | all versions up to commit TBD |
>> |Systems affected | All systems defining `CONFIG_IP_DEFRAG` |
>> |CVE identifier | TBD |
>> |Advisory URL | TBD |
>> |Risk | Critical 9.6 (CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H) |
>> |Authors | Nicolas Guigo, Nicolas Bidron |
>>
>> ### Summary
>>
>> U-boot is a popular boot loader for embedded systems with
>> implementations for a large number of architectures and prominent in
>> most linux based embedded systems.
>>
>> ### Location
>>
>> In `u-boot/net/net.c` the `__net_defragment` function line 900 through 1018.
>>
>> ### Impact
>>
>> The U-Boot implementation of
>> [RFC815](https://datatracker.ietf.org/doc/html/rfc815) IP DATAGRAM
>> REASSEMBLY ALGORITHMS is susceptible to a Hole Descriptor overwrite
>> attack which ultimately leads to an arbitrary write primitve.
>>
>> ### Description
>>
>> In compiled versions of U-Boot that define CONFIG_IP_DEFRAG, a value of
>> `ip->ip_len` (IP packet header's Total Length) higher than `IP_HDR_SIZE`
>> and strictly lower than `IP_HDR_SIZE+8` will lead to a value for `len`
>> comprised between `0` and `7`. This will ultimately result in a
>> truncated division by `8` resulting value of `0` forcing the hole
>> metadata and fragment to point to the same location. The subsequent
>> memcopy will overwrite the hole metadata with the fragment data. Through
>> a second fragment, this can be exploited to write to an arbitrary offset
>> controlled by that overwritten hole metadata value.
>>
>> This bug is only exploitable locally as it requires crafting two packets
>> the first of which would most likely be dropped through routing due to
>> its unexpectedly low Total Length. However, this bug can potentially be
>> exploited to root linux based embedded devices locally.
>>
>> ```C
>> static struct ip_udp_hdr *__net_defragment(struct ip_udp_hdr *ip, int *lenp)
>> {
>>       static uchar pkt_buff[IP_PKTSIZE] __aligned(PKTALIGN);
>>       static u16 first_hole, total_len;
>>       struct hole *payload, *thisfrag, *h, *newh;
>>       struct ip_udp_hdr *localip = (struct ip_udp_hdr *)pkt_buff;
>>       uchar *indata = (uchar *)ip;
>>       int offset8, start, len, done = 0;
>>       u16 ip_off = ntohs(ip->ip_off);
>>
>>       /* payload starts after IP header, this fragment is in there */
>>       payload = (struct hole *)(pkt_buff + IP_HDR_SIZE);
>>       offset8 =  (ip_off & IP_OFFS);
>>       thisfrag = payload + offset8;
>>       start = offset8 * 8;
>>       len = ntohs(ip->ip_len) - IP_HDR_SIZE;
>> ```
>>
>> The last line of the previous excerpt from `u-boot/net/net.c` shows how
>> the attacker can control the value of `len` to be strictly lower than
>> `8` by issuing a packet with `ip_len` between `21` and `27`
>> (`IP_HDR_SIZE` has a value of `20`).
>>
>> Also note that `offset8` here is `0` which leads to `thisfrag = payload`.
>>
>> ```C
>>       } else if (h >= thisfrag) {
>>           /* overlaps with initial part of the hole: move this hole */
>>           newh = thisfrag + (len / 8);
>>           *newh = *h;
>>           h = newh;
>>           if (h->next_hole)
>>               payload[h->next_hole].prev_hole = (h - payload);
>>           if (h->prev_hole)
>>               payload[h->prev_hole].next_hole = (h - payload);
>>           else
>>               first_hole = (h - payload);
>>
>>       } else {
>> ```
>>
>> Lower down the same function, execution reaches the above code path.
>> Here, `len / 8` evaluates to `0` leading to `newh = thisfrag`. Also note
>> that `first_hole` here is `0` since `h` and `payload` point to the same
>> location.
>>
>> ```C
>>       /* finally copy this fragment and possibly return whole packet */
>>       memcpy((uchar *)thisfrag, indata + IP_HDR_SIZE, len);
>> ```
>>
>> Finally, in the above excerpt the `memcpy` overwrites the hole metadata
>> since `thisfrag` and `h` both point to the same location. The hole
>> metadata is effectively overwritten with arbitrary data from the
>> fragmented IP packet data. If `len` was crafted to be `6`, `last_byte`,
>> `next_hole`, and `prev_hole` of the `first_hole` can be controlled by
>> the attacker.
>>
>> Finally the arbitrary offset write occurs through a second fragment that
>> only needs to be crafted to write data in the hole pointed to by the
>> previously controlled hole metadata (`next_hole`) from the first packet.
>>
>> ### Recommendation
>>
>> Handle cases where `len` is strictly lower than 8 by preventing the
>> overwrite of the hole metadata during the memcpy of the fragment. This
>> could be achieved by either:
>> * Moving the location where the hole metadata is stored when `len` is
>> lower than `8`.
>> * Or outright rejecting fragmented IP datagram with a Total Length
>> (`ip_len`) lower than 28 bytes which is the minimum valid fragmented IP
>> datagram size (as defined as the minimum fragment of 8 octets in the IP
>> Specification Document:
>> [RFC791](https://datatracker.ietf.org/doc/html/rfc791) page 25).
>>
>> ----------BUG 2----------
>>
>> # Large buffer overflow leads to DoS in U-Boot IP Packet Defragmentation
>> Code (CVE-TBD)
>>
>> |  |  |
>> | --- | --- |
>> |Project | U-Boot |
>> |Project URL | https://github.com/u-boot/u-boot |
>> |Versions affected | all versions up to commit TBD |
>> |Systems affected | All systems defining `CONFIG_IP_DEFRAG` |
>> |CVE identifier | TBD |
>> |Advisory URL | TBD |
>> |Risk | High 7.1 (CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:N/I:L/A:H) |
>> |Authors | Nicolas Guigo, Nicolas Bidron |
>>
>> ### Summary
>>
>> U-boot is a popular boot loader for embedded systems with
>> implementations for a large number of architectures and prominent in
>> most linux based embedded systems.
>>
>> ### Location
>>
>> `u-boot/net/net.c` lines 915 and 1011.
>>
>> ### Impact
>>
>> The U-Boot implementation of
>> [RFC815](https://datatracker.ietf.org/doc/html/rfc815) IP DATAGRAM
>> REASSEMBLY ALGORITHMS is susceptible to a buffer overflow through a
>> specially crafted fragmented IP Datagram with an invalid Total Length
>> which creates Denial of Service conditions.
>>
>> ### Description
>>
>> In compiled versions of U-Boot that define CONFIG_IP_DEFRAG, a value of
>> `ip->ip_len` (IP packet header's Total Length) lower than `IP_HDR_SIZE`
>> will lead to a negative value for `len` which will ultimately result in
>> a buffer overflow during the subsequent `memcpy` that uses `len` as it's
>> `count` parameter.
>>
>> This bug is only exploitable on local ethernet as it requires crafting
>> an invalid packet to include an unexpected `ip_len` value in the IP UDP
>> header that's lower than the minimum accepted Total Length of a packet
>> (21 as defined in the IP Specification Document:
>> [RFC791](https://datatracker.ietf.org/doc/html/rfc791)). Such packet
>> would in all likelihood be dropped while being routed to its final
>> destination through most routing equipment and as such requires the
>> attacker to be in a local position in order to be exploited.
>>
>> ```C
>> static struct ip_udp_hdr *__net_defragment(struct ip_udp_hdr *ip, int *lenp)
>> {
>>       static uchar pkt_buff[IP_PKTSIZE] __aligned(PKTALIGN);
>>       static u16 first_hole, total_len;
>>       struct hole *payload, *thisfrag, *h, *newh;
>>       struct ip_udp_hdr *localip = (struct ip_udp_hdr *)pkt_buff;
>>       uchar *indata = (uchar *)ip;
>>       int offset8, start, len, done = 0;
>>       u16 ip_off = ntohs(ip->ip_off);
>>
>>       /* payload starts after IP header, this fragment is in there */
>>       payload = (struct hole *)(pkt_buff + IP_HDR_SIZE);
>>       offset8 =  (ip_off & IP_OFFS);
>>       thisfrag = payload + offset8;
>>       start = offset8 * 8;
>>       len = ntohs(ip->ip_len) - IP_HDR_SIZE;
>> ```
>>
>> The last line of the previous excerpt from `u-boot/net/net.c` shows
>> where the undeflow to a negative `len` value occurs if `ip_len` is set
>> to a value strictly lower than 20 (`IP_HDR_SIZE` being 20). Also note
>> that in the above excerpt the `pkt_buff` buffer has a size of
>> `CONFIG_NET_MAXDEFRAG` which defaults to 16 KB but can range from 1KB to
>> 64 KB depending on configurations.
>>
>> ```C
>>       /* finally copy this fragment and possibly return whole packet */
>>       memcpy((uchar *)thisfrag, indata + IP_HDR_SIZE, len);
>> ```
>>
>> In the above excerpt the `memcpy` overflows the destination by
>> attempting to make a copy of nearly 4 gigabytes in a buffer that's
>> designed to hold `CONFIG_NET_MAXDEFRAG` bytes at most which leads to a DoS.
>>
>> ### Recommendation
>>
>> Stop processing of the packet if `ip_len` is lower than 21 (as defined
>> by the minimum length of a data carrying datagram in the IP
>> Specification Document:
>> [RFC791](https://datatracker.ietf.org/doc/html/rfc791) page 34).
>>
>>
>> Nicolas Bidron, Nicolas Guigo
>>
> Hi Nicolas,
> Thanks for the research.
> I have read your description thoroughly, very interesting.
> I will implement fixes to the findings.

Thanks for looking into this. I wanted to give it a shot as well but got 
side-tracked. Could you keep me in CC on any patch you have for this?

Thanks a lot.
Matthias

Re: Vulnerability Disclosure in net/

[Fabio Estevam]

Hi Ramon,

On Wed, May 25, 2022 at 11:46 PM Ramon Fried <rfried.dev@gmail.com> wrote:

> Hi Nicolas,
> Thanks for the research.
> I have read your description thoroughly, very interesting.
> I will implement fixes to the findings.

Is it enough to add the check below?

--- a/net/net.c
+++ b/net/net.c
@@ -906,6 +906,9 @@ static struct ip_udp_hdr *__net_defragment(struct
ip_udp_hdr *ip, int *lenp)
        uchar *indata = (uchar *)ip;
        int offset8, start, len, done = 0;
        u16 ip_off = ntohs(ip->ip_off);
+
+       if (ip->ip_len < 28)
+               return NULL;

        /* payload starts after IP header, this fragment is in there */
        payload = (struct hole *)(pkt_buff + IP_HDR_SIZE);

Re: Vulnerability Disclosure in net/

[Michael Nazzareno Trimarchi]

Hi Fabio

On Thu, May 26, 2022 at 2:13 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Ramon,
>
> On Wed, May 25, 2022 at 11:46 PM Ramon Fried <rfried.dev@gmail.com> wrote:
>
> > Hi Nicolas,
> > Thanks for the research.
> > I have read your description thoroughly, very interesting.
> > I will implement fixes to the findings.
>
> Is it enough to add the check below?
>
> --- a/net/net.c
> +++ b/net/net.c
> @@ -906,6 +906,9 @@ static struct ip_udp_hdr *__net_defragment(struct
> ip_udp_hdr *ip, int *lenp)
>         uchar *indata = (uchar *)ip;
>         int offset8, start, len, done = 0;
>         u16 ip_off = ntohs(ip->ip_off);
> +
> +       if (ip->ip_len < 28)
> +               return NULL;
>
If you comment on it up or nobody will remember what is 28 tomorrow

Michael


>         /* payload starts after IP header, this fragment is in there */
>         payload = (struct hole *)(pkt_buff + IP_HDR_SIZE);



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com