| Message ID | AeKyVq1-Mbokue6LkYrzI6shnnjBlZ8Jomlb2JUGY-y2QxhwJCqNin47ZVRITHcekmiYe0MwjTlYhv9Q6_gmwA0vDbkOxJ2Yp8PkoajfF-Y=@protonmail.com |
|---|---|
| State | Under Review |
| Headers | show |
| Series | Fix buffer overflow for 'Include' parameter in pacman.conf | expand |
On 10/1/22 20:40, tim174 wrote: > If the 'Include' parameter in the config file is set to a long string > (for example 3000x '/') the pacman config parser will crash when calling glob > in conf.c with this value. If the string is shorter than approx. 3000 symbols > the normal error message is printed and the segfault does not occur. > > I was able to reproduce this on my own system and on the official Arch Linux > iso image. The PATH_MAX variable is too large to prevent this bug, > hence the new variable GLOB_LIMIT with a security buffer of 1000 > since I don't know how persistent this limit is across systems. > Its origin is unclear to me and I am not sure if it is a fixed value. > This is why I would appreciate any help to make this a sustainable patch. > > Thanks > Tim > I guess this comes down to this section that is just below your change.... /* Ignore include failures... assume non-critical */ I'd assume you are hitting "GLOB_NOSPACE". We should probably give proper errors here and for "GLOB_ABORTED". Allan > Signed-off-by: Tim <tim174 at protonmail.com> > --- > src/pacman/conf.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/src/pacman/conf.c b/src/pacman/conf.c > index f9edf75b..d8222752 100644 > --- a/src/pacman/conf.c > +++ b/src/pacman/conf.c > @@ -65,6 +65,9 @@ config_t *config = NULL; > #define BOLDWHITE "\033[1;37m" > #define GREY46 "\033[38;5;243m" > > +/* limit for glob input variable */ > +#define GLOB_LIMIT 2000 > + > void enable_colors(int colors) > { > colstr_t *colstr = &config->colstr; > @@ -1042,6 +1045,13 @@ static int process_include(const char *value, void *data, > return 1; > } > > + if(strlen(value) > GLOB_LIMIT) { > + pm_printf(ALPM_LOG_ERROR, > + ("config file %s, line %d, directive '%s': value too long\n"), > + file, linenum, "Include"); > + return 1; > + } > + > if(section->depth >= config_max_recursion) { > pm_printf(ALPM_LOG_ERROR, > _("config parsing exceeded max recursion depth of %d.\n"), > -- > 2.34.1 > .
diff --git a/src/pacman/conf.c b/src/pacman/conf.c index f9edf75b..d8222752 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -65,6 +65,9 @@ config_t *config = NULL; #define BOLDWHITE "\033[1;37m" #define GREY46 "\033[38;5;243m" +/* limit for glob input variable */ +#define GLOB_LIMIT 2000 + void enable_colors(int colors) { colstr_t *colstr = &config->colstr; @@ -1042,6 +1045,13 @@ static int process_include(const char *value, void *data, return 1; } + if(strlen(value) > GLOB_LIMIT) { + pm_printf(ALPM_LOG_ERROR, + ("config file %s, line %d, directive '%s': value too long\n"), + file, linenum, "Include"); + return 1; + } + if(section->depth >= config_max_recursion) { pm_printf(ALPM_LOG_ERROR, _("config parsing exceeded max recursion depth of %d.\n"),
If the 'Include' parameter in the config file is set to a long string (for example 3000x '/') the pacman config parser will crash when calling glob in conf.c with this value. If the string is shorter than approx. 3000 symbols the normal error message is printed and the segfault does not occur. I was able to reproduce this on my own system and on the official Arch Linux iso image. The PATH_MAX variable is too large to prevent this bug, hence the new variable GLOB_LIMIT with a security buffer of 1000 since I don't know how persistent this limit is across systems. Its origin is unclear to me and I am not sure if it is a fixed value. This is why I would appreciate any help to make this a sustainable patch. Thanks Tim Signed-off-by: Tim <tim174 at protonmail.com> --- src/pacman/conf.c | 10 ++++++++++ 1 file changed, 10 insertions(+) -- 2.34.1