Security vulnerabilities report to Das-U-Boot

[Gao Xiang]

Hi Tom,

On 2025/2/11 00:41, Tom Rini wrote:
> On Fri, Feb 07, 2025 at 09:53:01AM -0800, Jonathan Bar Or wrote:
> 
>> 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.
> 
> Adding in the eorfs and squashfs maintainers..
> 
>>
>> Best regards,
>>              Jonathan
>>
>> On Fri, Feb 7, 2025 at 7:50 AM Tom Rini <trini at 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.

Yeah, it's a corner case, I will try to address it later.

Thanks,
Gao Xiang