The misc-progs utilities are often setuid and can be executed by the group "nobody". They use functions like "safe_system" and often do not perform input validation.
See for example:
As it stands, these binaries are a trivial way to elevate privileges should a vulnerable service be exploited.
I didn't see misc-progs in ipfire 3. However, since ipfire 2 seems to be the release that's offered for general use, this does effect ipfire users today.
In the following example, tested on the newest ipfire release, a user "test" has been created that's also a member of the group "nobody".
-su-5.0$ cat /etc/os-release
PRETTY_NAME="IPFire 2.25 (x86_64) - core153"
uid=1001(test) gid=1001(test) groups=1001(test),99(nobody)
-su-5.0$ /usr/local/bin/launch-ether-wake test ';/bin/bash;%'
/usr/sbin/etherwake: option requires an argument -- 'i'
usage: ether-wake [-i <ifname>] [-p aa:bb:cc:dd[:ee:ff]] 00:11:22:33:44:55
Use '-u' to see the complete set of options.
[root@ipfire test]# id
uid=0(root) gid=0(root) groups=0(root)
So, an attacker that takes control over a process that's in the nobody group, e. g. the httpd apache server or unbound, can trivially gain root access.
It doesn't look that ether-wake is the only command that's affected. It seems some commands in misc-progs try to filter user input, while others do not.
An obvious solution is to validate all inputs for the commands. So, in the above example, launch-ether-wake could check whether an interface exists (or whether the parameter consists of characters that are valid for interfaces) or whether the mac address is valid.
The comment for the safe_system function https://github.com/ipfire/ipfire-2.x/blob/master/src/misc-progs/setuid.c#L56 actually that you should filter for semicolons etc. Commands like backupctrl https://github.com/ipfire/ipfire-2.x/blob/master/src/misc-progs/backupctrl.c#L25 however don't filter for semicolons, only a few chars like &&. Therefore, the checks can probably be considered insufficient.
So, possible first steps towards improving the situation could be the following:
1. Implementing a function like safe_system(), except that it would not invoke system() but rather the binary directly, e. g. using the exec() family of functions. This avoids those injection commands. It should be used in all calls that only call a single command. In the example above, the exploit only works because safe_system() uses /bin/sh but there is no reason to call /bin/sh. Had it called etherwake directly, the exploit would not have worked.
2. Currently, some commands try to filter unwanted characters in argv before calling safe_system(), but miss some characters or don't have any filtering at all. Therefore, a central function could be implemented that would be used by all commands. Note however that this is still a blacklisting approach and thus not ideal.
3. Whitelist as much as possible. If a parameter is supposed to be a mac address, make sure it is one. Or if a parameter is expected to be only alphanumeric characters, check for this too.
Thank you for reporting this vulnerability. It helps us a lot to make IPFire more secure.
I will have a look at this and submit patches to this ticket. So far I can confirm it and I have marked this ticket at security-sensitive so that it won't be visible on the bug tracker. I will remove that mark as soon as a patch has been submitted and has been approved.
I have now come up with various fixes for these vulnerabilities which I would like to post here and I am asking for a review of them. If you agree with the changes, I will post them to the mailing list.
The lauch-ether-wake binary has been dropped and I have given etherwake CAP_NET_RAW. Therefore the web UI can now call it without need for the helper.
I have removed that commands that are composed from input variables are executed using a shell. Therefore the entire class of shell command injection should be removed. I have changed safe_system() so that it is a wrapper for the new function and not all of the code has to be changed.
After these changes, all vulnerabilities should be fixed. Please let me know if you find anything that I overlooked.
Thank you again for your help to make IPFire better software :)
Created attachment 831 [details]
Created attachment 832 [details]
Created attachment 833 [details]
Created attachment 834 [details]
Created attachment 835 [details]
Created attachment 836 [details]
Created attachment 837 [details]
Created attachment 838 [details]
Created attachment 839 [details]
Created attachment 840 [details]
Created attachment 841 [details]
Created attachment 842 [details]
Created attachment 843 [details]
Created attachment 844 [details]
Created attachment 845 [details]
Currently, I only have time for a quick review.
Patch 1: OK
Patch 2: OK
Patch 4: OK
Patch 5: OK
Patch 7: I personally would prefer calling run() instead of safe_system(), just as a matter of principle, since you only execute a single command here. Similarly, I would prefer snprintf() over sprintf(), even though you did check for overflows before. This way, people attention is not taken by these calls to sprintf() and analyzer will not produce warnings.
Patch 8: OK
Patch 9: OK
I'll have to check the rest once I have more time. To be clear, the "OK" only refers to this bug. So if the setuid binary calls a perl script, I did not check whether the perl script deals properly with the arguments or whether it has other bugs.
Created attachment 846 [details]
Thanks for looking into this so far.
(In reply to ipfire from comment #18)
> Patch 7: I personally would prefer calling run() instead of safe_system(),
> just as a matter of principle, since you only execute a single command here.
> Similarly, I would prefer snprintf() over sprintf(), even though you did
> check for overflows before. This way, people attention is not taken by these
> calls to sprintf() and analyzer will not produce warnings.
I replaced all of those calls with snprintf(). See the attached patch.
> I'll have to check the rest once I have more time. To be clear, the "OK"
> only refers to this bug. So if the setuid binary calls a perl script, I did
> not check whether the perl script deals properly with the arguments or
> whether it has other bugs.
This is what this ticket is about. I would like to release these fixes as soon as possible. If there is anything else in any of the scripts that are being called, that would be a new ticket and I would like to work on that after this. One at a time.
I would like to close the development cycle very soon and I would like these patches to be included to patch the problem as soon as possible for our users.
Did you have some more time to review them or do you need a little bit longer?
I have merged all patches into "next" and they are scheduled for release soon.
This bug report is also public again now.
Thank you for your help and your valuable contribution to IPFire.