[PATCH] conf: Fix off-by-one heap overrun in escape_chars()
escape_chars() computes the post-escaping buffer size as len + pattern_chars, but unfortunately never accounts for the null terminator it unconditionally writes afterwards. That means that when pacman-conf applies --sysroot to an Include directive and that sysroot path contains a character that also happens to be a glob metacharacter, the null terminator is written one byte past the end of the allocation. Fix both the overflow guard and the malloc size so the terminator always falls within the allocated region. The off by one write is reliably caught by ASAN, so add a dedicated TAP test to prevent regressions. Fixes: 7016adcb7035 ("manually apply --sysroot to configuration") --- meson.build | 2 +- src/pacman/conf.c | 4 +-- test/util/meson.build | 9 +++++ test/util/pacman-conf-sysroot-escape.sh | 45 +++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 3 deletions(-) create mode 100755 test/util/pacman-conf-sysroot-escape.sh diff --git a/meson.build b/meson.build index 8e2444d1..800f37e9 100644 --- a/meson.build +++ b/meson.build @@ -374,7 +374,7 @@ pacman_bin = executable( install : true, ) -executable( +pacman_conf_bin = executable( 'pacman-conf', pacman_conf_sources, include_directories : includes, diff --git a/src/pacman/conf.c b/src/pacman/conf.c index e391735a..fd419529 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -1132,8 +1132,8 @@ static char *escape_chars(const char *pattern, const char *escape) } /* allocate new string with overflow check */ - if(SIZE_MAX - len < pattern_chars - || !(escaped = malloc(len + pattern_chars))) { + if(SIZE_MAX - len <= pattern_chars + || !(escaped = malloc(len + pattern_chars + 1))) { errno = ENOMEM; return NULL; } diff --git a/test/util/meson.build b/test/util/meson.build index b4046298..71921296 100644 --- a/test/util/meson.build +++ b/test/util/meson.build @@ -5,3 +5,12 @@ test('vercmptest', args : [ join_paths(meson.current_source_dir(), 'vercmptest.sh') ]) + +test('pacman-conf-sysroot-escape', + BASH, + env : TEST_ENV, + protocol : 'tap', + args : [ + join_paths(meson.current_source_dir(), 'pacman-conf-sysroot-escape.sh') + ], + depends : [pacman_conf_bin]) diff --git a/test/util/pacman-conf-sysroot-escape.sh b/test/util/pacman-conf-sysroot-escape.sh new file mode 100755 index 00000000..473c369d --- /dev/null +++ b/test/util/pacman-conf-sysroot-escape.sh @@ -0,0 +1,45 @@ +#!/bin/bash +# +# pacman-conf-sysroot-escape - verify sysroot glob escaping under ASAN +# +# Copyright (c) 2026 Pacman Development Team <pacman-dev@lists.archlinux.org> +# +# This program is free software; you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free Software +# Foundation; either version 2 of the License, or (at your option) any later +# version. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program. If not, see <http://www.gnu.org/licenses/>. + +source "$(dirname "$0")"/../tap.sh || exit 1 + +bin=${1:-${PMTEST_UTIL_DIR}pacman-conf} + +if ! type -p "$bin" &>/dev/null; then + tap_bail "pacman-conf binary ($bin) could not be located" + exit 1 +fi + +tmpdir=$(mktemp -d) +trap 'rm -rf "$tmpdir"' EXIT + +sysroot="$tmpdir/sys[root" +mkdir -p "$sysroot/etc" "$sysroot/dev" +: > "$sysroot/dev/null" +printf '[options]\nInclude = /dev/null\n' > "$sysroot/etc/pacman.conf" + +tap_plan 2 + +output=$("$bin" --sysroot "$sysroot" --config /etc/pacman.conf RootDir 2>&1) +ret=$? + +tap_is_int "$ret" 0 "pacman-conf parses Include under metachar sysroot" +tap_is_str "$output" "$sysroot/" "pacman-conf preserves the escaped sysroot" + +tap_finish base-commit: a6f7467d8c7c4d7e9cc846884e74c0ab7215c48d -- 2.53.0
Chris Down writes:
escape_chars() computes the post-escaping buffer size as len + pattern_chars, but unfortunately never accounts for the null terminator it unconditionally writes afterwards. That means that when pacman-conf applies --sysroot to an Include directive and that sysroot path contains a character that also happens to be a glob metacharacter, the null terminator is written one byte past the end of the allocation.
Fix both the overflow guard and the malloc size so the terminator always falls within the allocated region.
The off by one write is reliably caught by ASAN, so add a dedicated TAP test to prevent regressions.
Fixes: 7016adcb7035 ("manually apply --sysroot to configuration")
Of course I forgot to put the Signed-off-by ;-) When committing please feel free to add: Signed-off-by: Chris Down <chris@chrisdown.name>
On 19/03/2026 3:07 pm, Chris Down wrote:
Chris Down writes:
escape_chars() computes the post-escaping buffer size as len + pattern_chars, but unfortunately never accounts for the null terminator it unconditionally writes afterwards. That means that when pacman-conf applies --sysroot to an Include directive and that sysroot path contains a character that also happens to be a glob metacharacter, the null terminator is written one byte past the end of the allocation.
Fix both the overflow guard and the malloc size so the terminator always falls within the allocated region.
The off by one write is reliably caught by ASAN, so add a dedicated TAP test to prevent regressions.
Fixes: 7016adcb7035 ("manually apply --sysroot to configuration")
Of course I forgot to put the Signed-off-by ;-)
When committing please feel free to add:
Signed-off-by: Chris Down <chris@chrisdown.name>
Patches sent to the mailing list get lost. Please submit to gitlab: https://gitlab.archlinux.org/pacman/pacman/
participants (2)
-
Allan McRae
-
Chris Down