From 0842fdfe1d070106af3852bcd31688422130405b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kha=C3=AFs=20COLIN?= Date: Tue, 1 Apr 2025 18:38:31 +0200 Subject: [PATCH] fix: double free when getting command path for empty command ``` minishell$ "" ``` leads to the following error: ``` ================================================================= ==1158770==ERROR: AddressSanitizer: attempting double-free on 0x5060000020c0 in thread T0: #0 0x5649fcb5b8a8 in free.part.0 asan_malloc_linux.cpp.o #1 0x5649fcbac549 in deal_with_filled_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:119:3 #2 0x5649fcbac38b in filepath_from_env /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:165:10 #3 0x5649fcbac220 in get_cmdpath /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:186:10 #4 0x5649fcba8664 in simple_cmd_execute /home/khais/src/42/common_env/minishell/src/executing/simple_cmd/simple_cmd_execute.c:71:8 #5 0x5649fcba6e4c in execute_command /home/khais/src/42/common_env/minishell/src/minishell.c:49:2 #6 0x5649fcba6ce5 in main /home/khais/src/42/common_env/minishell/src/minishell.c:92:3 #7 0x7fb4f2dcd27d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) #8 0x7fb4f2dcd338 in __libc_start_main@GLIBC_2.2.5 (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a338) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) #9 0x5649fca6f394 in _start (/home/khais/src/42/common_env/minishell/minishell+0x2b394) 0x5060000020c0 is located 0 bytes inside of 58-byte region [0x5060000020c0,0x5060000020fa) freed by thread T0 here: #0 0x5649fcb5b8a8 in free.part.0 asan_malloc_linux.cpp.o #1 0x5649fcbac669 in select_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:81:3 #2 0x5649fcbac524 in deal_with_filled_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:116:9 #3 0x5649fcbac38b in filepath_from_env /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:165:10 #4 0x5649fcbac220 in get_cmdpath /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:186:10 #5 0x5649fcba8664 in simple_cmd_execute /home/khais/src/42/common_env/minishell/src/executing/simple_cmd/simple_cmd_execute.c:71:8 #6 0x5649fcba6e4c in execute_command /home/khais/src/42/common_env/minishell/src/minishell.c:49:2 #7 0x5649fcba6ce5 in main /home/khais/src/42/common_env/minishell/src/minishell.c:92:3 #8 0x7fb4f2dcd27d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) previously allocated by thread T0 here: #0 0x5649fcb5c897 in malloc (/home/khais/src/42/common_env/minishell/minishell+0x118897) #1 0x5649fcbac6be in alloc_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp_utils.c:34:9 #2 0x5649fcbac4e9 in deal_with_filled_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:113:15 #3 0x5649fcbac38b in filepath_from_env /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:165:10 #4 0x5649fcbac220 in get_cmdpath /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:186:10 #5 0x5649fcba8664 in simple_cmd_execute /home/khais/src/42/common_env/minishell/src/executing/simple_cmd/simple_cmd_execute.c:71:8 #6 0x5649fcba6e4c in execute_command /home/khais/src/42/common_env/minishell/src/minishell.c:49:2 #7 0x5649fcba6ce5 in main /home/khais/src/42/common_env/minishell/src/minishell.c:92:3 #8 0x7fb4f2dcd27d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) SUMMARY: AddressSanitizer: double-free asan_malloc_linux.cpp.o in free.part.0 ==1158770==ABORTING ``` Looks like select_path frees filepath sometimes, no idea why. Removing these lines didn't break any tests. I'm pushing this to get a second opinion --- src/subst/simple_filename_exp.c | 4 +--- test.sh | 7 +++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/subst/simple_filename_exp.c b/src/subst/simple_filename_exp.c index 3e1451b..0f8bd66 100644 --- a/src/subst/simple_filename_exp.c +++ b/src/subst/simple_filename_exp.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/03/02 13:40:10 by jguelen #+# #+# */ -/* Updated: 2025/03/27 18:39:23 by khais ### ########.fr */ +/* Updated: 2025/04/01 18:38:35 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -77,8 +77,6 @@ static char *select_path(char *filepath, char **oldpath, char **path, else return (free(*oldpath), path_split_destroy(path), filepath); } - if (*oldpath != filepath) - free(filepath); return (NULL); } diff --git a/test.sh b/test.sh index e6fcefd..2b647bc 100755 --- a/test.sh +++ b/test.sh @@ -335,6 +335,13 @@ status=1 hello=hi EOF +when_run <