On 25.06.2025 02:05, Heinrich Schuchardt wrote: > [You don't often get email from heinrich.schucha...@canonical.com. > Learn why this is important at > https://aka.ms/LearnAboutSenderIdentification ] > > On 24.06.25 23:05, Mikhail Kshevetskiy wrote: >> >> On 24.06.2025 18:34, Heinrich Schuchardt wrote: >>> [You don't often get email from heinrich.schucha...@canonical.com. >>> Learn why this is important at >>> https://aka.ms/LearnAboutSenderIdentification ] >>> >>> A malformed FIT image could have an image name property that is not NUL >>> terminated. Reject such images. >>> >>> Reported-by: Mikhail Kshevetskiy <mikhail.kshevets...@iopsys.eu> >>> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com> >>> --- >>> common/spl/spl_fit.c | 10 ++++++++-- >>> 1 file changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c >>> index e250c11ebbd..25f3c822a49 100644 >>> --- a/common/spl/spl_fit.c >>> +++ b/common/spl/spl_fit.c >>> @@ -73,7 +73,7 @@ static int spl_fit_get_image_name(const struct >>> spl_fit_info *ctx, >>> const char **outname) >>> { >>> struct udevice *sysinfo; >>> - const char *name, *str; >>> + const char *name, *str, *end; >>> __maybe_unused int node; >>> int len, i; >>> bool found = true; >>> @@ -83,11 +83,17 @@ static int spl_fit_get_image_name(const struct >>> spl_fit_info *ctx, >>> debug("cannot find property '%s': %d\n", type, len); >>> return -EINVAL; >>> } >>> + /* A string property should be NUL terminated */ >>> + end = name + len - 1; >>> + if (!len || *end) { >>> + debug("malformed property '%s'\n", type); >>> + return -EINVAL; >>> + } >>> >>> str = name; >>> for (i = 0; i < index; i++) { >>> str = strchr(str, '\0') + 1; >>> - if (!str || (str - name >= len)) { >>> + if (str > end) { >> >> and if strchr() will return NULL, then str will be equal to 1. In this >> case str will be less then end, so the loop will not terminate. > > strchr() searching for NUL will never return NULL but search until it > hits NUL. > > And as the patch adds checks that the buffer pointed to by str is NUL > terminated we will not read outside of the buffer.
Ok, good from my side > > Best regards > > Heinrich > >> >> >>> found = false; >>> break; >>> } >>> -- >>> 2.48.1 >>> >
