From fb9acedcadb0a05a60c5feeb36d9ec70e4e2bd77 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 26 May 2020 21:42:13 +0200 Subject: [PATCH] libpkgconf: dependency: fix out of boundary write It is possible to trigger an out of boundary write in function pkgconf_dependency_parse_str if a dependency line contains a very long comparator. The comparator is stored in a temporary buffer which has a size of PKGCONF_ITEM_SIZE. The line which is parsed can be up to PKGCONF_BUFSIZE characters long, which is larger than PKGCONF_ITEM_SIZE (although it depends on PATH_MAX). Having a comparator which is longer than PKGCONF_ITEM_SIZE therefore leads to an out of boundary write. Although it is undefined behaviour, this can lead to an overridden compare variable, which in turn can lead to an invalid instruction pointer, i.e. most likely a crash or code execution (very unlikely). Proof of concept: $ echo "Requires: x " > poc.pc $ dd if=/dev/zero bs=1 count=65535 | tr '\0' '<' >> poc.pc $ pkgconf poc.pc Eiter compile pkgconf with address sanitizer or run pkgconf multiple times, eventually it might crash (assuming that ASLR is in place). In order to fix this, I decided to use an end pointer to avoid OOB write. Alternative would be to increase the buffer size, but I try to avoid that since this would be additional ~60 KB stack space for a very unlikely situation. --- libpkgconf/dependency.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libpkgconf/dependency.c b/libpkgconf/dependency.c index b3722b1..d5cdf3c 100644 --- a/libpkgconf/dependency.c +++ b/libpkgconf/dependency.c @@ -237,6 +237,7 @@ pkgconf_dependency_parse_str(const pkgconf_client_t *client, pkgconf_list_t *dep char *vstart = NULL; char *package = NULL, *version = NULL; char *cnameptr = cmpname; + char *cnameend = cmpname + PKGCONF_ITEM_SIZE - 1; memset(cmpname, '\0', sizeof cmpname); @@ -304,7 +305,8 @@ pkgconf_dependency_parse_str(const pkgconf_client_t *client, pkgconf_list_t *dep if (PKGCONF_IS_OPERATOR_CHAR(*ptr)) { state = INSIDE_OPERATOR; - *cnameptr++ = *ptr; + if (cnameptr < cnameend) + *cnameptr++ = *ptr; } break; @@ -315,7 +317,7 @@ pkgconf_dependency_parse_str(const pkgconf_client_t *client, pkgconf_list_t *dep state = AFTER_OPERATOR; compare = pkgconf_pkg_comparator_lookup_by_name(cmpname); } - else + else if (cnameptr < cnameend) *cnameptr++ = *ptr; break;