Hi Bastien,
On Thu, Jul 4, 2024 at 6:33 AM Bastien Nocera <hadess@xxxxxxxxxx> wrote:
>
> Error: RESOURCE_LEAK (CWE-772): [#def40] [important]
> bluez-5.76/src/shared/shell.c:1113:3: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv".
> bluez-5.76/src/shared/shell.c:1114:4: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
> 1112|
> 1113| if (wordexp(rl_line_buffer, &w, WRDE_NOCMD))
> 1114|-> return NULL;
Derr, this is NOCMD has been found...
> 1115|
> 1116| matches = menu_completion(default_menu, text, w.we_wordc,
>
> Error: RESOURCE_LEAK (CWE-772): [#def42] [important]
> bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv".
> bluez-5.76/src/shared/shell.c:1415:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
> 1413| switch (err) {
> 1414| case WRDE_BADCHAR:
> 1415|-> return -EBADMSG;
> 1416| case WRDE_BADVAL:
> 1417| case WRDE_SYNTAX:
Ok, but where in the documentation of wordexp it is saying that
we_wordv is left with anything allocated if it fails? Ive assumed if
it returns an error no argument has been processed, otherwise this is
sort of misleading and the errors shall be returned by index of the
word.
> Error: RESOURCE_LEAK (CWE-772): [#def43] [important]
> bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv".
> bluez-5.76/src/shared/shell.c:1418:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
> 1416| case WRDE_BADVAL:
> 1417| case WRDE_SYNTAX:
> 1418|-> return -EINVAL;
> 1419| case WRDE_NOSPACE:
> 1420| return -ENOMEM;
>
> Error: RESOURCE_LEAK (CWE-772): [#def44] [important]
> bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv".
> bluez-5.76/src/shared/shell.c:1420:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
> 1418| return -EINVAL;
> 1419| case WRDE_NOSPACE:
> 1420|-> return -ENOMEM;
> 1421| case WRDE_CMDSUB:
> 1422| if (wordexp(input, &w, 0))
>
> Error: RESOURCE_LEAK (CWE-772): [#def45] [important]
> bluez-5.76/src/shared/shell.c:1422:3: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv".
> bluez-5.76/src/shared/shell.c:1423:4: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
> 1421| case WRDE_CMDSUB:
> 1422| if (wordexp(input, &w, 0))
> 1423|-> return -ENOEXEC;
> 1424| break;
> 1425| };
> ---
> src/shared/shell.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/shell.c b/src/shared/shell.c
> index 878be140c336..c09d41ee54df 100644
> --- a/src/shared/shell.c
> +++ b/src/shared/shell.c
> @@ -1117,8 +1117,10 @@ static char **shell_completion(const char *text, int start, int end)
> if (start > 0) {
> wordexp_t w;
>
> - if (wordexp(rl_line_buffer, &w, WRDE_NOCMD))
> + if (wordexp(rl_line_buffer, &w, WRDE_NOCMD)) {
> + wordfree(&w);
> return NULL;
> + }
>
> matches = menu_completion(default_menu, text, w.we_wordc,
> w.we_wordv[0]);
> @@ -1421,15 +1423,20 @@ int bt_shell_exec(const char *input)
> err = wordexp(input, &w, WRDE_NOCMD);
> switch (err) {
> case WRDE_BADCHAR:
> + wordfree(&w);
> return -EBADMSG;
> case WRDE_BADVAL:
> case WRDE_SYNTAX:
> + wordfree(&w);
> return -EINVAL;
> case WRDE_NOSPACE:
> + wordfree(&w);
> return -ENOMEM;
> case WRDE_CMDSUB:
> - if (wordexp(input, &w, 0))
> + if (wordexp(input, &w, 0)) {
> + wordfree(&w);
> return -ENOEXEC;
> + }
> break;
> };
If we really need to call wordfree regardless then I'd probably have a
function that wraps wordexp and automatically does wordfree on error.
--
Luiz Augusto von Dentz
