11–20 of 46 posts
> 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…
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.
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 e…
Hmm, wonder if this is the type of stuff a static code analysis would pick up like (veracode)
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) U…
Instead php should have an interface that accepts:
exec(["sudo", "pihole", "-a", "addstaticdhcp", $mac, $ip, $hostname]);
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.
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 e…
Hmm, wonder if this is the type of stuff a static code analysis would pick up like (veracode)
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 int…
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]);> 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…
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 e…
Hmm, wonder if this is the type of stuff a static code analysis would pick up like (veracode)