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

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.

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.

What is bmc software?

idrac, ilo, etc.

Yes definitely. My ISP does not allow me to expose a DNS server to the internet, they told me it was against the law. So it seems you'd have to break the law in my country in order to be vulnerable to this!

Huh, really? Against the law, or against the contract? In what country do you live?

I think it's more than 'pretty low issue'. Someone already connected to the LAN may just sniff the login credentials since the Pi-hole web interface doesn't use https and then gain root access where all LAN clients trust with their DNS queries.

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.

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!

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

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`…

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.

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]);

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()

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.

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

> 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.

Using sudo in the exec() isn't really a problem, since any RCE an attacker would get will have the same privileges as the pihole service...they could just use sudo in their RCE if the user is in sudousers (assuming they could even use sudo on their bash session when establishing the reverse shell). In the case of the PoC here, sudo doesn't even come into play, they are stacking commands with && which would require another sudo call to elevate (which they don't do in the example).

So find the rest and fix them

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?

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

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.

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.

Code review on a open source project....

Mind you they trusted a regex that looks pretty sane (A preg_replace might of been better)

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?

There's a csrf token set in a php session that's hard to guess that needs to be provided in form data, yes.

Probably difficult to exploit that way without first finding another bug to retrieve that token from a random origin.

Requires a POST that would hit CORS pretty hard

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...

dns rebinding could make it work, thats not the question

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?

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.

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

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.

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.

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.

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

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.

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?

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.

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.