On 28/04/2024 08:34, Hank Leininger wrote:
> On 2024-04-27, Jacob Bachmeyer wrote:
>>> - Check for irregular contents in .pc files, inspired by Vegard
>>> Nossum's oss-security post
>
>> Much easier: look for pkg-config descriptions containing text
>> other than a variable definition. The pkg-config tool itself
>> should probably enforce "cleanliness" on this matter and refuse to
>> process files containing other text. (It also should complain
>> about and reject an *-uninstalled.pc file found in the system
>> directories, which was another logic error exploited in that sample
>> backdoor.)
>
> Really, doing this seems a more robust approach anyway, because
> allowing only known-good > rejecting known-bad. I was mostly driven
> by "hang on, how many of the things Nossum's example does are
> actually used by real files?" and the answer from my initial sample
> size was zero, so it'd be trivial to extend that check to every .pc
> file shipped by every current distro's packages.
>
> I think Sam looked into existing pkg-config verifiers and found they
> do not complain about things we thought they should complain about
> (this could just mean we misunderstand their purpose). A strict
> lint-checker for such files would be better than just checking for
> specific suspicious patterns. But, I don't yet know how strict a
> format we could insist on (would it turn out 10% of files in fact
> break what we initially think are reasonable rules?). Even still, I
> think you could embed badness in legit variables, although I haven't
> dug in enough to know that for sure.
Hi,
Masquerading a shell command as a pkg-config variable definition is
trivial (but probably still detectable) since you can just do:
foobar=/usr echo hi
which AFAIK is a valid pkg-config variable definition but also a valid
shell command.
Also remember that in my particular example I reused the same file but
it would also be trivial to use a different file in the $(...) expansion
so that the payload actually lives somewhere else. The payload doesn't
even have to be a shell script, it could also be a small ELF binary or
something where you wouldn't necessarily be able to tell at a glance
that it does something malicious.
So probably the real thing to look for would be $(...) in pkg-config
files -- Hank, you mentioned in the GitHub issue that you did fine this
in one file; out of curiosity, could you share it?
I tried this on my system and didn't find anything:
$ grep -R '\$(' /usr/share/pkgconfig /usr/lib/x86_64-linux-gnu/pkgconfig
It's also worth asking if there are other ways to encode that $() that
bypasses the very simple '\$(' pattern -- e.g. something like "$\(" or
maybe an expansion of a variable that itself contains the $ character:
$ cat test.pc
foo=\$
Name: test
Version: 0
Description:
Cflags: ${foo}(echo hi)
$ PKG_CONFIG_PATH=. pkg-config --cflags test
$(echo hi)
There are also other ways to achieve the same effect.
I should also add that I found out-of-bounds memory accesses in both the
original pkg-config and pkgconf (used on Debian and RedHat derivatives,
respectively, AFAIK) when using long variable names -- it doesn't look
exploitable to me but I've submitted some patches for both packages just
in case.
Thanks,
Vegard
Vegard Nossum wrote:
> [...]
> Hi,
>
> Masquerading a shell command as a pkg-config variable definition is
> trivial (but probably still detectable) since you can just do:
>
> foobar=/usr echo hi
>
> which AFAIK is a valid pkg-config variable definition but also a valid
> shell command.
You are correct, but making this a little bit harder for an attacker is
still an improvement. Perhaps pkg-config variable values should be
required to be in quotes if they contain spaces?
The bigger issue is accepting an *-uninstalled.pc in a system directory,
which means that it actually *has* been installed. That logic error
allowed your backdoor to override the real libelf.pc without producing a
file conflict that the package manager could detect.
> Also remember that in my particular example I reused the same file but
> it would also be trivial to use a different file in the $(...) expansion
> so that the payload actually lives somewhere else.
Agreed, but adding another file to the backdoor increases the chance of
the attacker getting caught.
> The payload doesn't
> even have to be a shell script, it could also be a small ELF binary or
> something where you wouldn't necessarily be able to tell at a glance
> that it does something malicious.
Also correct, in fact, for a package that actually installs executables,
a bit of extra code in an otherwise legitimate binary to detect when the
grandparent is make(1) and drop a backdoor could very likely go
unnoticed. (This would be the rogue or compromised distribution
packager scenario, where the binaries distributed do not match the sources.)
-- Jacob