Hi Anton,
On Sat, Feb 01, 2025 at 01:43:44AM +0300, Anton Moryakov wrote:
> Report of the static analyzer:
> After having been compared to a NULL value at
> elflint.c:252, pointer 'suffix' is dereferenced at elflint.c:260
> by calling function 'stpcpy'
>
> Corrections explained:
> When processing a file with a NULL suffix, the code could dereference
> a NULL pointer, leading to undefined behavior. This patch adds a check
> to ensure suffix is not NULL before using it in stpcpy.
>
> The fix ensures that new_suffix is properly initialized even when
> suffix is NULL, avoiding potential crashes.
>
> Triggers found by static analyzer Svace.
Thanks, this analyzis is correct locally in this process_file
function.
But in practice this (static) function is called either with both
prefix and suffix as initialized (stack) pointers:
char new_prefix[prefix_len + 1 + fname_len];
char new_suffix[(suffix == NULL ? 0 : strlen (suffix)) + 2];
[...]
process_file (fd, subelf, new_prefix, new_suffix,
arhdr->ar_name, arhdr->ar_size, false);
Or with both prefix and suffix NULL:
process_file (fd, elf, NULL, NULL, argv[remaining], st.st_size,
only_one);
So the code path where prefix != NULL also implies suffix != NULL.
Maybe the code needs a comment, check or assert to verify this?
Cheers,
Mark
> Signed-off-by: Anton Moryakov <ant.v.moryakov at gmail.com>
>
> ---
> src/elflint.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/elflint.c b/src/elflint.c
> index cdc6108d..fba18f5a 100644
> --- a/src/elflint.c
> +++ b/src/elflint.c
> @@ -257,7 +257,10 @@ process_file (int fd, Elf *elf, const char *prefix,
> const char *suffix,
> {
> cp = mempcpy (cp, prefix, prefix_len);
> *cp++ = '(';
> - strcpy (stpcpy (new_suffix, suffix), ")");
> + if(suffix != NULL)
> + strcpy (stpcpy (new_suffix, suffix), ")");
> + else
> + new_suffix[0] = '\0';
> }
> else
> new_suffix[0] = '\0';
> --
> 2.30.2
>
