| Message ID | e542f40a-8f73-7ab7-11af-2c62263a1897@gmx.de |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Greetings On 11/9/19 6:18 PM, Tim Rühsen wrote: > In line > if (dirs[num_dirs - 1] == NULL) > 'num_dirs' can be 0. > > Regards, Tim > I've looked at the control-flow on this file and I do not see a possible path for num_dirs to be 0 at line 186, unless the file name in question is the empty string "", at which point it may just be easier to return earlier. Can you please verify this? Thank you. Cheers, Orlando.
On 10.11.19 05:51, Orlando Arias wrote: > Greetings > > On 11/9/19 6:18 PM, Tim Rühsen wrote: >> In line >> if (dirs[num_dirs - 1] == NULL) >> 'num_dirs' can be 0. >> >> Regards, Tim >> > > I've looked at the control-flow on this file and I do not see a possible > path for num_dirs to be 0 at line 186, unless the file name in question > is the empty string "", at which point it may just be easier to return > earlier. Can you please verify this? Yes, I can confirm. Thanks for review ! Returning early on an empty 'name' also fixes another read overflow in #ifdef HAVE_DOS_BASED_FILE_SYSTEM if (name[1] == ':' && IS_DIR_SEPARATOR (name[2])) { p += 3; num_dirs++; } #endif /* HAVE_DOS_BASED_FILE_SYSTEM */ Patch v2 appended. Regards, Tim From 848cf430c4393555f35f4e4d27da8a555f6308c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20R=C3=BChsen?= <tim.ruehsen@gmx.de> Date: Sun, 10 Nov 2019 11:51:00 +0100 Subject: [PATCH] [libiberty] Fix read buffer overflow in split_directories() * libiberty/make-relative-prefix.c: Return early on empty 'name' --- libiberty/make-relative-prefix.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libiberty/make-relative-prefix.c b/libiberty/make-relative-prefix.c index ec0b0ee749..2ff2af8a59 100644 --- a/libiberty/make-relative-prefix.c +++ b/libiberty/make-relative-prefix.c @@ -122,6 +122,9 @@ split_directories (const char *name, int *ptr_num_dirs) const char *p, *q; int ch; + if (!*name) + return NULL; + /* Count the number of directories. Special case MSDOS disk names as part of the initial directory. */ p = name; -- 2.24.0
Greetings, On 11/10/19 5:54 AM, Tim Rühsen wrote: > Yes, I can confirm. Thanks for review ! Thank you for checking. Please note that I am not a binutils maintainer [or reviewer for that matter]. I am just an interested party. Having said that > Returning early on an empty 'name' also fixes another read overflow in > > #ifdef HAVE_DOS_BASED_FILE_SYSTEM > if (name[1] == ':' && IS_DIR_SEPARATOR (name[2])) > { > p += 3; > num_dirs++; > } > #endif /* HAVE_DOS_BASED_FILE_SYSTEM */ > > Patch v2 appended. > I believe you also need to do something about the ptr_num_dirs that the caller may work with at some point. It would also be good to check how this function is called within binutils [and other projects that use libiberty]. Thank you for your time and efforts. Cheers, Orlando.
On 10.11.19 18:33, Orlando Arias wrote: > Greetings, > > On 11/10/19 5:54 AM, Tim Rühsen wrote: >> Yes, I can confirm. Thanks for review ! > > Thank you for checking. Please note that I am not a binutils maintainer > [or reviewer for that matter]. I am just an interested party. Having > said that > >> Returning early on an empty 'name' also fixes another read overflow in >> >> #ifdef HAVE_DOS_BASED_FILE_SYSTEM >> if (name[1] == ':' && IS_DIR_SEPARATOR (name[2])) >> { >> p += 3; >> num_dirs++; >> } >> #endif /* HAVE_DOS_BASED_FILE_SYSTEM */ >> >> Patch v2 appended. >> > > I believe you also need to do something about the ptr_num_dirs that the > caller may work with at some point. It would also be good to check how > this function is called within binutils [and other projects that use > libiberty]. > > Thank you for your time and efforts. Thank you for looking into it :-) The callers of split_directories() check for the return value. If NULL, they 'goto bailout' (cleanup and return), and ptr_num_dirs is definitely not used. Regards, Tim
Hi Tim, > Yes, I can confirm. Thanks for review ! > Patch v2 appended. Just to be clear: patches for libiberty need to be sent to gcc-patches@gcc.gnu.org rather than this mailing list. (You can of course discuss the patches here if you want to). The libiberty library is maintained by gcc rather then the binutils... Cheers Nick
Hi Nick, On 11/11/19 12:42 PM, Nick Clifton wrote: >> Patch v2 appended. > > Just to be clear: patches for libiberty need to be sent to gcc-patches@gcc.gnu.org > rather than this mailing list. (You can of course discuss the patches here if you > want to). The libiberty library is maintained by gcc rather then the binutils... Thanks, this is clear since one of your last emails - I oversaw that in the README. Sorry :-) Regards, Tim
From 65ca7c0688cb823ebde141c7cf74075db7cd0aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20R=C3=BChsen?= <tim.ruehsen@gmx.de> Date: Sun, 10 Nov 2019 00:12:44 +0100 Subject: [PATCH] [libiberty] Fix read buffer overflow in split_directories() * libiberty/make-relative-prefix.c: Check array index before use --- libiberty/make-relative-prefix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libiberty/make-relative-prefix.c b/libiberty/make-relative-prefix.c index ec0b0ee749..d53304eca7 100644 --- a/libiberty/make-relative-prefix.c +++ b/libiberty/make-relative-prefix.c @@ -186,7 +186,7 @@ split_directories (const char *name, int *ptr_num_dirs) dirs[num_dirs++] = save_string (q, p - 1 - q); dirs[num_dirs] = NULL; - if (dirs[num_dirs - 1] == NULL) + if (num_dirs && dirs[num_dirs - 1] == NULL) { free_split_directories (dirs); return NULL; -- 2.24.0