It is possible (but unlikely) to encounter very long symbolic
links, which would take a few mega bytes in size. In general,
such long symbolic links are impossible to create because the
kernel will only accept up to PATH_MAX in length. BUT lstat()
on a maliciously modified symbolic link can return a much
larger size than that.
In function check_file_link (src/pacman/check.c), the length
of a symbolic link is used in a variable length array:
char link[length]; // length being st->st_size + 1
This doesn't just allow a very theoretical overflow on 32 bit
systems with 64 bit off_t (which is st_size's type), it can also
overflow available stack space, leading to a segmentation fault.
Symbolic links pointing to something that is longer than PATH_MAX
can be considered illegal, so just get a fixed size array and
reject symbolic links which are longer than that.
According to gnulib, buggy file systems can report garbage in
st_size, so there is no benefit in checking length against it.
Signed-off-by: Tobias Stoeckmann <tob...@stoeckmann.org>
---
src/pacman/check.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/pacman/check.c b/src/pacman/check.c
index d282cc2..db6b423 100644
--- a/src/pacman/check.c
+++ b/src/pacman/check.c
@@ -130,17 +130,18 @@ static int check_file_time(const char *pkgname, const
char *filepath,
}
static int check_file_link(const char *pkgname, const char *filepath,
- struct stat *st, struct archive_entry *entry)
+ struct archive_entry *entry)
{
- size_t length = st->st_size + 1;
- char link[length];
+ ssize_t length;
+ char link[PATH_MAX];
- if(readlink(filepath, link, length) != st->st_size) {
+ length = readlink(filepath, link, PATH_MAX);
+ if(length == -1 || length >= PATH_MAX) {
/* this should not happen */
pm_printf(ALPM_LOG_ERROR, _("unable to read symlink contents:
%s\n"), filepath);
return 1;
}
- link[length - 1] = '\0';
+ link[length] = '\0';
if(strcmp(link, archive_entry_symlink(entry)) != 0) {
if(!config->quiet) {
@@ -348,7 +349,7 @@ int check_pkg_full(alpm_pkg_t *pkg)
file_errors += check_file_permissions(pkgname, filepath, &st,
entry);
if(type == AE_IFLNK) {
- file_errors += check_file_link(pkgname, filepath, &st,
entry);
+ file_errors += check_file_link(pkgname, filepath,
entry);
}
/* the following checks are expected to fail if a backup file
has been
--
2.8.3
