mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* Heap overflow vulnerabilities in network implementation of barebox
@ 2024-05-23 16:51 jianqiang wang
  2024-05-27  7:41 ` Sascha Hauer
  0 siblings, 1 reply; 3+ messages in thread
From: jianqiang wang @ 2024-05-23 16:51 UTC (permalink / raw)
  To: barebox

Dear Barebox devlopers,

I found several heap overflow vulnerabilities in Barebox.

The Barebox implementation assumes that the network packet received is
less than PKTSIZE, that is 1536 bytes. For example, the /net/net.c
file ping_reply function assumes that the packet received is 1536
bytes and allocates a 1536 bytes buffer then copies the packet data
into the buffer.

However, in the driver layer, it lacks a proper check of the packet length.
For example, in drivers/net/cs8900.c cs8900_probe function, it
allocates a PKTSIZE buffer and assigns it to rx_buf. In cs8900_recv
function, the length is read from the device register:

len = readw(priv->regs + CS8900_RTDATA0);

After that, the data is read from the register in a loop without a
boundary check.
The same vulnerability happens to the following drivers:

drivers/net/ks8851_mll.c function ks8851_rx_frame, it only and the
packet length with RXFHBCR_CNT_MASK (4095 bytes,) which is not
consistent with the upper layer length check.

drivers/net/liteeth.c function liteeth_eth_rx, It checks if the length
is larger than 2048 which is inconsistent with the upper layer.

drivers/net/smc911x.c function smc911x_eth_rx. The packet length is
read from the register without checking.

It would be good to add a proper and consistent boundary check for
these drivers otherwise it will lead to potential heap overflow
vulnerability.

Best regards



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Heap overflow vulnerabilities in network implementation of barebox
  2024-05-23 16:51 Heap overflow vulnerabilities in network implementation of barebox jianqiang wang
@ 2024-05-27  7:41 ` Sascha Hauer
  2024-05-27  9:45   ` jianqiang wang
  0 siblings, 1 reply; 3+ messages in thread
From: Sascha Hauer @ 2024-05-27  7:41 UTC (permalink / raw)
  To: jianqiang wang; +Cc: barebox

Hi,

On Thu, May 23, 2024 at 06:51:01PM +0200, jianqiang wang wrote:
> Dear Barebox devlopers,
> 
> I found several heap overflow vulnerabilities in Barebox.
> 
> The Barebox implementation assumes that the network packet received is
> less than PKTSIZE, that is 1536 bytes. For example, the /net/net.c
> file ping_reply function assumes that the packet received is 1536
> bytes and allocates a 1536 bytes buffer then copies the packet data
> into the buffer.
> 
> However, in the driver layer, it lacks a proper check of the packet length.
> For example, in drivers/net/cs8900.c cs8900_probe function, it
> allocates a PKTSIZE buffer and assigns it to rx_buf. In cs8900_recv
> function, the length is read from the device register:
> 
> len = readw(priv->regs + CS8900_RTDATA0);
> 
> After that, the data is read from the register in a loop without a
> boundary check.
> The same vulnerability happens to the following drivers:
> 
> drivers/net/ks8851_mll.c function ks8851_rx_frame, it only and the
> packet length with RXFHBCR_CNT_MASK (4095 bytes,) which is not
> consistent with the upper layer length check.
> 
> drivers/net/liteeth.c function liteeth_eth_rx, It checks if the length
> is larger than 2048 which is inconsistent with the upper layer.
> 
> drivers/net/smc911x.c function smc911x_eth_rx. The packet length is
> read from the register without checking.
> 
> It would be good to add a proper and consistent boundary check for
> these drivers otherwise it will lead to potential heap overflow
> vulnerability.

Thanks for noting this. I've just sent a series fixing the drivers you
explicitly mentioned. Additionally I have checked a few other drivers
and it seems at least the smc91111 driver has this issue as well. Are
you aware of other drivers?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Heap overflow vulnerabilities in network implementation of barebox
  2024-05-27  7:41 ` Sascha Hauer
@ 2024-05-27  9:45   ` jianqiang wang
  0 siblings, 0 replies; 3+ messages in thread
From: jianqiang wang @ 2024-05-27  9:45 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi Sascha,

Thanks for your work. I noticed that if the device does not use DMA,
it will probably have this problem. Yes, what you mentioned the
smc91111 driver has the same vulnerability. However, I didn't check
them carefully for each device driver.

Best


Sascha Hauer <s.hauer@pengutronix.de> 于2024年5月27日周一 09:41写道:
>
> Hi,
>
> On Thu, May 23, 2024 at 06:51:01PM +0200, jianqiang wang wrote:
> > Dear Barebox devlopers,
> >
> > I found several heap overflow vulnerabilities in Barebox.
> >
> > The Barebox implementation assumes that the network packet received is
> > less than PKTSIZE, that is 1536 bytes. For example, the /net/net.c
> > file ping_reply function assumes that the packet received is 1536
> > bytes and allocates a 1536 bytes buffer then copies the packet data
> > into the buffer.
> >
> > However, in the driver layer, it lacks a proper check of the packet length.
> > For example, in drivers/net/cs8900.c cs8900_probe function, it
> > allocates a PKTSIZE buffer and assigns it to rx_buf. In cs8900_recv
> > function, the length is read from the device register:
> >
> > len = readw(priv->regs + CS8900_RTDATA0);
> >
> > After that, the data is read from the register in a loop without a
> > boundary check.
> > The same vulnerability happens to the following drivers:
> >
> > drivers/net/ks8851_mll.c function ks8851_rx_frame, it only and the
> > packet length with RXFHBCR_CNT_MASK (4095 bytes,) which is not
> > consistent with the upper layer length check.
> >
> > drivers/net/liteeth.c function liteeth_eth_rx, It checks if the length
> > is larger than 2048 which is inconsistent with the upper layer.
> >
> > drivers/net/smc911x.c function smc911x_eth_rx. The packet length is
> > read from the register without checking.
> >
> > It would be good to add a proper and consistent boundary check for
> > these drivers otherwise it will lead to potential heap overflow
> > vulnerability.
>
> Thanks for noting this. I've just sent a series fixing the drivers you
> explicitly mentioned. Additionally I have checked a few other drivers
> and it seems at least the smc91111 driver has this issue as well. Are
> you aware of other drivers?
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-05-27  9:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-23 16:51 Heap overflow vulnerabilities in network implementation of barebox jianqiang wang
2024-05-27  7:41 ` Sascha Hauer
2024-05-27  9:45   ` jianqiang wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox