[pacman-dev] [PATCH] be_package: validate package file paths
Overly long paths cannot be extracted and paths with newlines cannot be represented in our database format. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/be_package.c | 15 ++++++++++++--- test/pacman/tests/TESTS | 3 +++ test/pacman/tests/filename-basename-too-long.py | 15 +++++++++++++++ test/pacman/tests/filename-path-too-long.py | 20 ++++++++++++++++++++ test/pacman/tests/filename-with-newline.py | 11 +++++++++++ 5 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 test/pacman/tests/filename-basename-too-long.py create mode 100644 test/pacman/tests/filename-path-too-long.py create mode 100644 test/pacman/tests/filename-with-newline.py diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 53399a3..52db319 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -18,6 +18,7 @@ * along with this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <limits.h> #include <stdlib.h> #include <string.h> #include <errno.h> @@ -390,7 +391,17 @@ static int add_entry_to_files_list(alpm_filelist_t *filelist, const size_t files_count = filelist->count; alpm_file_t *current_file; mode_t type; - size_t pathlen; + size_t pathlen = strlen(path); + + /* +2 to leave space for prepending minimal possible root and appending + * trailing slash if a directory */ + if(pathlen + 2 >= PATH_MAX || strlen(mbasename(path)) >= NAME_MAX) { + return -1; + } + /* our database format cannot represent paths with newlines */ + if(memchr(path, '\n', pathlen)) { + return -1; + } if(!_alpm_greedy_grow((void **)&filelist->files, files_size, (files_count + 1) * sizeof(alpm_file_t))) { @@ -399,8 +410,6 @@ static int add_entry_to_files_list(alpm_filelist_t *filelist, type = archive_entry_filetype(entry); - pathlen = strlen(path); - current_file = filelist->files + files_count; /* mtree paths don't contain a tailing slash, those we get from diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index 8ad1b9c..210134f 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -50,6 +50,9 @@ TESTS += test/pacman/tests/fileconflict025.py TESTS += test/pacman/tests/fileconflict030.py TESTS += test/pacman/tests/fileconflict031.py TESTS += test/pacman/tests/fileconflict032.py +TESTS += test/pacman/tests/filename-basename-too-long.py +TESTS += test/pacman/tests/filename-path-too-long.py +TESTS += test/pacman/tests/filename-with-newline.py TESTS += test/pacman/tests/hook-abortonfail.py TESTS += test/pacman/tests/hook-file-change-packages.py TESTS += test/pacman/tests/hook-file-remove-trigger-match.py diff --git a/test/pacman/tests/filename-basename-too-long.py b/test/pacman/tests/filename-basename-too-long.py new file mode 100644 index 0000000..93a14a5 --- /dev/null +++ b/test/pacman/tests/filename-basename-too-long.py @@ -0,0 +1,15 @@ +import os + +self.description = "Install packages with overly long file names" + +filename = 'A' * (os.pathconf(self.rootdir(), 'PC_NAME_MAX') + 1) + +p = pmpkg("foo") +p.files = [filename] +self.addpkg(p) + +self.args = "-U %s" % p.filename() + +self.addrule("!PACMAN_RETCODE=0") +self.addrule("!PKG_EXIST=foo") +self.addrule("!FILE_EXIST=%s" % filename) diff --git a/test/pacman/tests/filename-path-too-long.py b/test/pacman/tests/filename-path-too-long.py new file mode 100644 index 0000000..c4278b5 --- /dev/null +++ b/test/pacman/tests/filename-path-too-long.py @@ -0,0 +1,20 @@ +import os + +self.description = "Install packages with overly long file paths" + +# construct a path > PATH_MAX with the fewest possible components to avoid +# having to build giant filelists and deep recursion in rmtree() +name_max = os.pathconf(self.rootdir(), 'PC_NAME_MAX') - 1 +path_max = os.pathconf(self.rootdir(), 'PC_PATH_MAX') +basename = 'A' * name_max + '/' +filename = basename * ((path_max / name_max) + 1) + +p = pmpkg("foo") +p.files = [filename] +self.addpkg(p) + +self.args = "-U %s" % p.filename() + +self.addrule("!PACMAN_RETCODE=0") +self.addrule("!PKG_EXIST=foo") +self.addrule("!FILE_EXIST=%s" % filename) diff --git a/test/pacman/tests/filename-with-newline.py b/test/pacman/tests/filename-with-newline.py new file mode 100644 index 0000000..df412db --- /dev/null +++ b/test/pacman/tests/filename-with-newline.py @@ -0,0 +1,11 @@ +self.description = "Install a package including a file with a newline in its name" + +p = pmpkg("foo") +p.files = ["foo\nbar"] +self.addpkg(p) + +self.args = "-U %s" % p.filename() + +self.addrule("!PACMAN_RETCODE=0") +self.addrule("!PKG_EXIST=foo") +self.addrule("!FILE_EXIST=foo\nbar") -- 2.6.2
On 03/11/15 04:04, Andrew Gregory wrote:
Overly long paths cannot be extracted and paths with newlines cannot be represented in our database format.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/be_package.c | 15 ++++++++++++--- test/pacman/tests/TESTS | 3 +++ test/pacman/tests/filename-basename-too-long.py | 15 +++++++++++++++ test/pacman/tests/filename-path-too-long.py | 20 ++++++++++++++++++++ test/pacman/tests/filename-with-newline.py | 11 +++++++++++ 5 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 test/pacman/tests/filename-basename-too-long.py create mode 100644 test/pacman/tests/filename-path-too-long.py create mode 100644 test/pacman/tests/filename-with-newline.py
diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 53399a3..52db319 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -18,6 +18,7 @@ * along with this program. If not, see <http://www.gnu.org/licenses/>. */
+#include <limits.h> #include <stdlib.h> #include <string.h> #include <errno.h> @@ -390,7 +391,17 @@ static int add_entry_to_files_list(alpm_filelist_t *filelist, const size_t files_count = filelist->count; alpm_file_t *current_file; mode_t type; - size_t pathlen; + size_t pathlen = strlen(path); + + /* +2 to leave space for prepending minimal possible root and appending + * trailing slash if a directory */ + if(pathlen + 2 >= PATH_MAX || strlen(mbasename(path)) >= NAME_MAX) { + return -1; + } + /* our database format cannot represent paths with newlines */ + if(memchr(path, '\n', pathlen)) { + return -1; + }
Are these worth of debug messages?
if(!_alpm_greedy_grow((void **)&filelist->files, files_size, (files_count + 1) * sizeof(alpm_file_t))) { @@ -399,8 +410,6 @@ static int add_entry_to_files_list(alpm_filelist_t *filelist,
type = archive_entry_filetype(entry);
- pathlen = strlen(path); - current_file = filelist->files + files_count;
/* mtree paths don't contain a tailing slash, those we get from
participants (2)
-
Allan McRae
-
Andrew Gregory