Thank you.
So, I'm attaching my findings in a md file - see attachment.
All of those could be avoided by using safe math, such as
__builtin_mul_overflow and __builtin_add_overflow, which are used in some
modules in Das-U-Boot.
There are many cases where seemingly unsafe addition and multiplication can
cause integer overflows, but not all are exploitable - I believe the ones I
report here are.
Let me know your thoughts.
Best regards,
Jonathan
On Fri, Feb 7, 2025 at 7:50 AM Tom Rini <tr...@konsulko.com> wrote:
> On Thu, Feb 06, 2025 at 07:47:54PM -0800, Jonathan Bar Or wrote:
>
> > Dear U-boot maintainers,
> >
> > What is the best way of reporting security vulnerabilities (memory
> > corruption issues) to Das-U-Boot? Is there a PGP key I should be using?
> > I have 4 issues that I think are worth fixing (with very easy fixes).
> >
> > Best regards,
> > Jonathan
>
> Hey. As per https://docs.u-boot.org/en/latest/develop/security.html
> please post them to the list in public. If you have possible solutions
> for them as well that's even better. Thanks!
>
> --
> Tom
>
Filesystem-based Das-U-Boot issues.
== erofs
=== Integer overflow leading to buffer overflow in symlink resolution
In file `fs.c`, when resolving symlinks, the function `erofs_off_t` gets an `erofs_inode` argument and performs a lookup on the symlink.
The function blindly trusts the `i_size` member of the input as such:
```c
size_t len = vi->i_size;
char *target;
int err;
target = malloc(len + 1);
if (!target)
return -ENOMEM;
target[len] = '\0';
err = erofs_pread(vi, target, len, 0);
if (err)
goto err_out;
```
The `erofs_inode` structure's `i_size` member is defined with the type `erofs_off_t` (which is a 64-bit unsigned integer).
Thereofre, if supplied as 0xFFFFFFFFFFFFFFFF, the `len + 1` input to `malloc` would overflow to 0, allocating a chunk with 0.
That chunk (saved in `target`) is later written with `erofs_pread`, overriding the chunk with partial data controlled by an attacker.
Therefore, we will have a heap buffer overflow due to an integer overflow in `len` calculation.
== squashfs
=== Integer overflow leading to buffer overflow in inode table parsing
In file `sqfs.c`, function `sqfs_read_inode_table` is responsible of reading an inode table.
It gets the superblock (attacker controlled) from the context. Then, it employs the following logic:
```c
n_blks = sqfs_calc_n_blks(sblk->inode_table_start, sblk->directory_table_start, &table_offset);
/* Allocate a proper sized buffer (itb) to store the inode table */
itb = malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
if (!itb)
return -ENOMEM;
if (sqfs_disk_read(start, n_blks, itb) < 0) {
ret = -EINVAL;
goto free_itb;
}
```
=== Integer overflow leading to buffer overflow in directory table parsing
Similarly to the previous issue in inode table parsing in `sqfs.c`, the same unsafe multiplication exists within the function `sqfs_read_directory_table` responsible for reading the directory table:
```c
n_blks = sqfs_calc_n_blks(sblk->directory_table_start,
sblk->fragment_table_start, &table_offset);
/* Allocate a proper sized buffer (dtb) to store the directory table */
dtb = malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
if (!dtb)
return -ENOMEM;
if (sqfs_disk_read(start, n_blks, dtb) < 0)
goto out;
```
The multiplication of `n_blks` and the block size (attacker-controlled 64-bit unsigned integer) is unsafe and might overflow, resulting in an out-of-bounds write.
=== Integer overflow leading to buffer overflow in nested file reading
Similarly to the previous issue in inode table parsing in `sqfs.c`, the same unsafe multiplication exists within the function `sqfs_read_nest` responsible for reading a file:
```c
data_buffer = malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
if (!data_buffer) {
ret = -ENOMEM;
goto out;
}
ret = sqfs_disk_read(start, n_blks, data_buffer);
```
A similar issue exists in the same function also later on:
```c
fragment = malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
if (!fragment) {
ret = -ENOMEM;
goto out;
}
ret = sqfs_disk_read(start, n_blks, fragment);
if (ret < 0)
goto out;
```
