Unhide icon indicating copy to clipboard operation
Unhide copied to clipboard

Parsing error leads to false positives when processes with long command lines exist

Open basak opened this issue 10 months ago • 5 comments

In Ubuntu, our packaging of unhide supplies some tests that merely check that various invocations of unhide exit with a zero status, as would be expected on a regular uncompromised system.

However, these tests seemed to fail in our infrastructure.

It turns out that in unhide-linux-compound.c:checkallreverse there is a fixed buffer allocation of char read_line[1024], and the parser makes no accommodation for the output of ps --no-header -eL o lwp,cmd resulting in a line that is greater than 1023 bytes. If this happens, then the parser attempts to read the next entry from the middle of the previous line. If the previous line happened to contain a 0 at an unfortunate position for example, then the parser will read as if a pid exists of 0, and fail to find it, resulting in a false positive.

This affected Ubuntu infrastructure because our test environment includes a process with a command line of >1023 bytes, causing the our unhide test runs to erroneously fail.

A quick hack is to increase the size of the buffer and I'll patch the Ubuntu package with this for now, but really the parser needs to be able to handle any line length that the underlying ps command might produce.

basak avatar Apr 18 '24 13:04 basak

BTW, the error checking of atol is also non-functional. atol returns 0 on error, not -1, and setting syspids to -1 in the previous line is redundant as the atol call unconditionally overwrites it.

basak avatar Apr 18 '24 13:04 basak

Hi @basak,

Nice catch, thanks for the report. So Ubuntu has a vicious test with a command line that exceeds 1023 characters in length, the 1024th of which is a number! Nice :). I replaced fgets() with getline(), my quick test shows it resolve the bad parsing.

I also fix same kind of problem in unhide-linux.c:printbadpid() where process cmdline, link, comm andadditional process info were truncated to 1000 characters.

My bad: The last time I used atol() it was with a very old libc (a pre-C99 version for on-board equipment) that left the return value unchanged if nothing could be converted. I should have read a recent doc. It's corrected too and I add a warning message if this case happens and verbose output is selected.

I also fix same kind of problem in unhide-linux.c:printbadpid() where process cmdline, link, comm andadditional process info were truncated to 1000 characters.

Could you verify the problem is corrected in your use case with the unhide-linux.c and unhide-linux-compound.c in the attached archive? corrected_parser.tar.gz

If everything is OK for you, I will soon do a new release. Bye.

patrick-g2 avatar Apr 21 '24 12:04 patrick-g2

Thank you for the fast response!

So Ubuntu has a vicious test with a command line that exceeds 1023 characters in length, the 1024th of which is a number!

This was mostly an accident! You can see some of the failure logs here. The very long command being run (that fires off the entire test mechanism) is on line 3.

I'm sorry I'm not really confident in being able to reliably reproduce the failure. I ended up studying the code to infer the problem.

We did manage to run a separate process along the lines of sleep $(python3 -c 'print("0" * 1048 + "100")') to trip the issue though. Perhaps you could try that against the old and new code?

basak avatar Apr 21 '24 18:04 basak

I had already tested using a script which injects fake line in the ps output. old code gives: Test #4 Call ps, but add a faked process. This should show "my_false_proc" repeated enough times for a length > 1023 as faked process Unhide 20211016 Copyright © 2010-2021 Yago Jesus & Patrick Gouin License GPLv3+ : GNU GPL version 3 or later http://www.unhide-forensics.info NOTE : This version of unhide is for systems using Linux >= 2.6 Used options: [*]Searching for Fake processes by verifying that all threads seen by ps are also seen by others Found FAKE PID: 65535 Command = my_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_proc00 not seen by 12 sys fonc Found FAKE PID: 0 Command = _false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_proc not seen by 3 sys fonc

New code gives: Test #4 Call ps, but add a faked process. This should show "my_false_proc" repeated enough times for a length > 1023 as faked process Unhide 20211016 Copyright © 2010-2021 Yago Jesus & Patrick Gouin License GPLv3+ : GNU GPL version 3 or later http://www.unhide-forensics.info NOTE : This version of unhide is for systems using Linux >= 2.6 Used options: [*]Searching for Fake processes by verifying that all threads seen by ps are also seen by others Found FAKE PID: 65535 Command = my_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_proc0000_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_procmy_false_proc not seen by 12 sys fonc

As you can see no more PID of 0, and the cmd line of the fake process is not truncated. So I was pretty confidant, the correction works as intended.

I just wanted you to verify in real conditions but it's no big deal if you can't test it :)

I've seen some other warnings to fix before pushing a new version. I will do it ASAP. By the way, do you apply one or more patches to unhide for Ubuntu? If so, would it be interesting to apply them upstream?

Regards.

patrick-g2 avatar Apr 21 '24 20:04 patrick-g2

No problem. Thanks!

By the way, do you apply one or more patches to unhide for Ubuntu? If so, would it be interesting to apply them upstream?

You can see all our patches here. You'll see that I extended the buffer as a stopgap for the imminent Ubuntu release, but clearly properly fixing it upstream is better and that's why I filed this issue instead. The other patches are done by Debian and Ubuntu inherited them unmodified. Feel free to take any of these or modify them as needed!

basak avatar Apr 21 '24 20:04 basak