libpkgconf: tuple: fix out of boundary write #197

ariadne · 2020-05-30T22:53:38Z

stoeckmann commented

2020-05-30 22:53:38 +00:00

(Migrated from github.com)

This is the same issue which has been fixed in dependency code.

If a line contains a variable which is longer than PKGCONF_ITEM_SIZE,
then the varname buffer overflows.

The code itself still does not check if a closing } exists and
truncates variable names which are too long. Since these would
be functional changes and this commit is about a protection against
undefined behaviour on a language level, these changes are not
included.

Proof of concept:
$ echo "Description: poc" > poc.pc
$ echo "Version: 1" >> poc.pc
echo -n 'Name:{'
$ dd if=/dev/zero bs=1 count=66535 | tr '\0' 'x' >> poc.pc
$ echo >> poc.pc
$ pkgconf poc.pc

On my Linux system, when compiled with gcc, the varname buffer overflows
directly into buf, which means that no crash can be notified.

It's easiest to figure out when adding strlen() and sizeof() output
as debug lines.

This is the same issue which has been fixed in dependency code. If a line contains a variable which is longer than PKGCONF_ITEM_SIZE, then the varname buffer overflows. The code itself still does not check if a closing } exists and truncates variable names which are too long. Since these would be functional changes and this commit is about a protection against undefined behaviour on a language level, these changes are not included. Proof of concept: $ echo "Description: poc" > poc.pc $ echo "Version: 1" >> poc.pc $ echo -n 'Name: ${' $ dd if=/dev/zero bs=1 count=66535 | tr '\0' 'x' >> poc.pc $ echo >> poc.pc $ pkgconf poc.pc On my Linux system, when compiled with gcc, the varname buffer overflows directly into buf, which means that no crash can be notified. It's easiest to figure out when adding strlen() and sizeof() output as debug lines.

ariadne commented

2020-05-31 01:01:26 +00:00

Thanks for this. I have made a note to generate a diagnostic for too long variable lengths.

stoeckmann commented

2020-05-31 07:22:53 +00:00

(Migrated from github.com)

Weird that this pull request is still open. Since the changes have been applied, I close this one on my own.