libpkgconf: dependency: fix out of boundary write #194

Closed
stoeckmann wants to merge 1 commits from dependency into master
stoeckmann commented 2020-05-26 19:58:45 +00:00 (Migrated from github.com)

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.

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.
stoeckmann commented 2020-05-26 20:02:39 +00:00 (Migrated from github.com)

Just a side note: I am not into doing this right after a release on purpose. Just mentioning issues when I find them (and time to fix them).

Just a side note: I am not into doing this right after a release on purpose. Just mentioning issues when I find them (and time to fix them).

hahahahahahahahaha its ok

hahahahahahahahaha its ok

I merged this one too.

I merged this one too.

Pull request closed

Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: ariadne/pkgconf#194
There is no content yet.