
Pi-hole Remote Code Execution - ingve
https://natedotred.wordpress.com/2020/03/28/cve-2020-8816-pi-hole-remote-code-execution/
======
jrwr
1) You have be be logged into the pi-hole 2) Requires the user to post into a
MAC Address field

While I agree this should be fixed. This is a pretty low issue

~~~
userbinator
AFAIK Pi-holes are almost always going to be sitting inside a LAN and owned by
whoever owns the other devices on the network too, so the risk is quite low.

~~~
bubblethink
In general, yes, but this is how real issues start. Look at all the bmc
software written in the world. It's utter horseshit. You are supposed to use a
dedicated vlan for accessing the bmc. Everyone is still fighting the bmc
software and it is routinely accessed over the open internet.

~~~
rany_
What is bmc software?

~~~
bubblethink
idrac, ilo, etc.

------
Gravyness
The dev trusted a regexp to 'validate' user input for a _privileged_ command
execution, a function which fails at validating a constant-sized, colon-
separated sequence of hex numbers in a string, everything about the input
screams structure, and yet it was still half-assed!

Although this requires you to authenticate at the web portal, so 'some' sort
of trust is necessary to gain this level of access. I believe they even have
some plan of letting authenticated users update the pi-hole code from the web
interface by the wording on this issue: [https://discourse.pi-hole.net/t/how-
do-i-update-pi-hole/249](https://discourse.pi-hole.net/t/how-do-i-update-pi-
hole/249).

A funny thought: you can create a script (that uses the default password) to
inject a code to schedule an update to the pi-hole (and revert the changed
permissions) which fixes the vulnerability and leaves no trace! These
possibilities reminds me of a hacker series!

~~~
ct520
Hmm, wonder if this is the type of stuff a static code analysis would pick up
like (veracode)

~~~
eswat
Likely would. The `validMAC` function doesn’t just fail silently if the regex
fails, which would get past naive signature detections. Oh and feeding user
input directly into `exec`…

------
mappu
The crazy thing is that there is a correct escaping available:
`escapeshellarg()`. With correct escaping, you don't even need to sanitize the
user input here - the target command will parse the MAC address itself and
report any errors.

The existing code is analogous to building SQL queries using string
concatenation and forgetting the mysqli_real_escape_string() call. Really the
solution is to use a parameterized interface (e.g. by calling pcntl_fork() +
pcntl_exec(), that accepts an array of arguments instead of a string command-
line).

Pcntl doesn't seem to have a wrapper for the posix_spawn syscall.

~~~
kpcyrd
escapeshellarg is an ugly hack, php is passing everything through `sh -c` for
no reason.

The pcntl_exec function is the only one that has a reasonable interface, but
is way too low level.

php should simply accept this as a valid function call:

    
    
        exec(["sudo", "pihole", "-a", "addstaticdhcp", $mac, $ip, $hostname]);

------
peter_d_sherman
The root of this issue seems to be a regexp function (preg_match()) in PHP
coupled with an exec() of a variable that was not properly screened, coupled
with an 'sudo' inside of the exec(), e.g.:

exec(" _sudo_ pihole -a addstaticdhcp ".$mac." ".$ip." ".$hostname);

and/or

exec(" _sudo_ pihole -a removestaticdhcp ".$mac);

So three places to audit:

1) Regexp's and related complex high-level functions;

2) Calls to exec()

3) Uses of sudo within an exec()

~~~
kpcyrd
The problem is that php expects a string that is then passed to sh -c, which
is then parsed by a shell.

Instead php should have an interface that accepts:

    
    
        exec(["sudo", "pihole", "-a", "addstaticdhcp", $mac, $ip, $hostname]);
    

without any shell trickery.

If there's a sudo in this specific like doesn't matter, your reverse shell is
going to run as a regular user and after that it only matters low locked down
the sudoers configuration is.

~~~
kilburn
Definitely not ideal, but php provides the escapeshellarg [1] function you are
supposed to use in these situations.

You can even implement a "safe" exec function like so:

    
    
      function safe_exec(array $args, array &$output = null, int &$return_var = 0) : string {
        return exec(
          implode(' ',
            array_map('escapeshellarg', $args)
          ), $output, $return_var
        );
      }
      
      // Example usage
      $output = [];
      $return_var = 0;
      echo safe_exec(['ls', '/tmp'], $output, $return_var);
      var_export($output);
      echo "Exit code: $return_var\n";
    

[1]
[https://www.php.net/manual/en/function.escapeshellarg.php](https://www.php.net/manual/en/function.escapeshellarg.php)

------
droithomme
> exec("sudo ... " . $user_input_string ...)

Wow, this is always a mistake and a huge one. exec() is dangerous, exec
calling with sudo more so, and should never be used in conjunction with
unprivileged user input like this. Granted a weak attempt was made to sanitize
the user string, but so weak one might wonder if it is Underhanded Code at
play here.

The big problem with this sort of issue is that it indicates that there almost
certainly are massive security problems elsewhere in this code base since this
one is low hanging fruit that never should have made it past even rudimentary
code review by anyone with a bare minimum knowledge of security.

~~~
corndoge
So find the rest and fix them

~~~
droithomme
I'd be happy to help you with this. My rate is $235/hr, minimum 30 hours. When
can I expect your deposit so we can get started?

~~~
Disposition
Yes, how dare he ask you to donate some of your time to contribute back to the
open source community, tsk.

~~~
droithomme
Wow you created a brand new account just to post that one comment? I'm
honored.

On to the response. First, doing something I don't want to do that someone
else told me to do and not getting paid for it is slavery. Slavery is bad.

Second, I don't use their solution. I have my own custom DNS intercept system
I wrote myself which is much better and also enjoys security by obscurity.
With a single user it's hardly worth the time to mess with.

Third, I already donated to their project, above. I reviewed their code,
agreed it was complete shit, and concurred with the consensus that they need a
complete security audit. That is extremely valuable advice which is worth
$3000. So I donated $3000 and all you've done is sit and whine and create
anonymous coward accounts to troll people. Tsk.

~~~
Disposition
It's funny that I'm apparently the troll here, your comment just made me
consider finally making an account.

If somebody prompts you to do something on the internet do you give it any
concern? You must be swimming in free iPhone X's then. He suggested you donate
time to an open source project which is about as equivalent to slavery as a
cashier at Burger King trying to upsell you a large whopper menu.

You've donated absolutely nothing to the project by commenting here if you
didn't provide the feedback directly via the projects public tools.

I actively contribute to many open source projects. Writing a shell script to
generate dnsmasq hosts files isn't exactly rocket science. It doesn't matter
if you don't use the project, lesser informed people do, by improving it you
improve a large amount of people's security. Call it virtual herd immunity, it
affects you too indirectly.

------
0x0
Does this pihole admin interface perform any kind of CSRF protection, or can
this be exploites by any random website as long as the victim has a browser
tab with a valid session?

~~~
jrwr
Requires a POST that would hit CORS pretty hard

~~~
deathanatos
Not all POST requests require a CORS preflight. In particular, "normal" form
submissions, like the type that are accessed via $_POST in the article, do
not.

Specifically, "simple requests" do not, covered here[1]. This case is POST,
with a Content-Type of application/x-www-form-urlencoded. (The rule of thumb
you can use is that these were all essentially possible to send prior to CORS
even existing. To send a POST, you would construct a form tag with the inputs
filled in with the desired values, set the target URL to the "victim" site,
and have JS submit the form.)

[1]: [https://developer.mozilla.org/en-
US/docs/Web/HTTP/CORS#Simpl...](https://developer.mozilla.org/en-
US/docs/Web/HTTP/CORS#Simple_requests)

------
solarkraft
You have to be an authenticated user on the admin portal. Doesn't this already
come with the expectation of being able to fully control the device?

~~~
scaglio
Not necessarily, web GUI password and user/root ones can be different. From an
attacker's point of view: he just needs to crack the Wi-fi (e.g. aircrack-ng),
join the LAN an then bruteforce the Pi-Hole webadmin password.

~~~
vgb2k18
I read further up [1] that Pi-Hole auth is http not https, so if you cracked
the WiFi aready, sniffing the webadmin password might yield a quicker return
than brute-forcing (depending on strength of password, and owner's login
frequency). Assuming the maximum password length of PiHoles is not < 20 chars.

[1]
[https://news.ycombinator.com/item?id=22717938](https://news.ycombinator.com/item?id=22717938)

------
needle0
Why the sudden use of the "Black Jack ni Yoroshiku" manga comic art? I know
the author released the entire thing with permissions to freely use the art
anywhere, but still.

~~~
quickthrowman
A family member of mine was the original author of pi-hope. He sold the
company to his partner last year, so any changes in aesthetics would be due to
the change in ownership.

~~~
promofaux
One of the Pi-hole dev's here. The comment you are replying to is talking
about the blog post about this CVE, not Pi-hole's aesthetics. (Which haven't
changed, btw)

Say hi to J from me.

~~~
quickthrowman
Ahh, my mistake. I will say hi for you :)

------
INTPenis
This is justification for people like me who rolled our own using the lists
from pihole and a tiny bit of shell script. No need for a web gui.

------
mg794613
Why is it that, instead of just forking and making a patch is nowadays
replaced with a CVE focus. Are CVE's the new big thing on your CV?

~~~
scaglio
The patch has been released the day after the issue was being noticed to Pi-
hole, as you can read at the bottom of the article. This is just an
informative article and, as a pentester, I find it very interesting.

------
mro_name
self-hosting and remote code execution don't go well together. In that very
moment it's not apt for non-tech users which is the main target group AFAIK.
Very sad. Just too much complexity.

