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.
---
 src/pacman/check.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/pacman/check.c b/src/pacman/check.c
index d282cc2..6e8cda7 100644
--- a/src/pacman/check.c
+++ b/src/pacman/check.c
@@ -132,15 +132,16 @@ 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)
 {
-       size_t length = st->st_size + 1;
-       char link[length];
+       ssize_t len;
+       char link[PATH_MAX];
 
-       if(readlink(filepath, link, length) != st->st_size) {
+       len = readlink(filepath, link, sizeof(link));
+       if (len == -1 || len >= 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[len] = '\0';
 
        if(strcmp(link, archive_entry_symlink(entry)) != 0) {
                if(!config->quiet) {
-- 
2.8.3

Reply via email to