[PATCH 1/3] common/spl: fix potential out of buffer access in spl_fit_get_image_name function

[Tom Rini]

On Fri, Jun 06, 2025 at 10:35:22PM +0300, Mikhail Kshevetskiy wrote:

> The current code have two issues:
> 1) ineffective NULL pointer check
> 
> 	str = strchr(str, '\0') + 1
> 	if (!str || ...
> 
>    The str here will never be NULL (because we add 1 to result of strchr())
> 
> 2) strchr() may go out of the buffer for the special forms of name variable.
>    It's better use memchr() function here.
> 
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy at iopsys.eu>
> ---
>  common/spl/spl_fit.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 86506d6905c..ab277bb2baa 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -86,11 +86,12 @@ static int spl_fit_get_image_name(const struct spl_fit_info *ctx,
>  
>  	str = name;
>  	for (i = 0; i < index; i++) {
> -		str = strchr(str, '\0') + 1;
> -		if (!str || (str - name >= len)) {
> +		str = memchr(str, '\0', name + len - str);
> +		if (!str) {
>  			found = false;
>  			break;
>  		}
> +		str++;
>  	}

Looking at the loop, I'm not sure why we're doing what we're doing here.

It's possible I'm missing something else here, but, fdt_getprop is going
to return us the data part of an fdt_property struct. When index is 0,
the normal case, we don't loop here at all? Then if the index is 1, my
head starts hurting. It's unclear if the case where we don't have a
string value returned but find the node works, or maybe it does because
we always try the next hunk of code?

But I think we should try and clean up what's done here now that we're
looking at it right now. Thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250606/1cb2753c/attachment.sig>