Unhide
Unhide copied to clipboard
Parsing error leads to false positives when processes with long command lines exist
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.
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.
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.
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?
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.
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!