From Config Update to Root Shell in FOSSBilling
The most dangerous bugs I find are rarely in the code that looks scary. They're in the code that looks like a formality, the kind nobody reviews twice because it's "just writing a config file."
A boring function, on purpose
I was doing a source review of FOSSBilling, an open-source billing platform a lot of small hosting providers run to manage invoices and client accounts. I wasn't hunting for anything specific. I was reading through the admin API surface the way I usually do: module by module, asking what each endpoint is allowed to touch and whether the code trusts anything it shouldn't.
One function stopped me: Config::prettyPrintArrayToPHP(). Most apps store settings in JSON or a database row. FOSSBilling writes its config out as literal PHP source, a file that gets included on every single request. That's an unusual design choice, and unusual choices are exactly where I slow down.
The line that shouldn't exist
Here's the string-serialization branch, more or less verbatim:
if (is_string($value)) {
return " => '{$value}'," . PHP_EOL;
}No escaping. None. The value gets dropped straight between two single quotes and written into a file that PHP will execute. I've seen this exact mistake in junior code before, just usually not shipping in production billing software with database credentials sitting a few lines above it in the same file.
$value can contain a single quote, it isn't a string value anymore. It's a line of PHP.I traced where a value like that could actually reach this function from the outside world, and found set_interface_ip(), an admin endpoint meant to let an operator change the server's bound IP address. It reads interface from the request, hands it straight to Config::setProperty(), and that eventually calls the vulnerable serializer. No validation. No escaping. Just a string, in, and a rewritten PHP file, out.
Turning a quote into a shell
The payload writes itself once you see the shape of the bug. Something like:
x']; passthru($_GET['c']); //Substituted into the template, that closes the string, closes the array literal, terminates the return statement, and hands PHP a brand-new statement to execute, a passthru() call reading a command from the query string. The trailing // comments out whatever was supposed to follow. It's syntactically clean. PHP has no idea anything went wrong.
From there the chain is almost anticlimactic: log in as any admin, POST the payload to set_interface_ip, then send any request at all to the server. config.php runs on every page load, so the shell is live immediately, and it stays live even after the admin session that planted it is gone. Revoking credentials doesn't clean up a file already sitting on disk.
Why "just an admin bug" undersells it
It's tempting to write this off because it needs admin access first. I don't think that holds up. Admin panels get exposed to the internet more often than anyone wants to admit, credential stuffing against them is routine, and the point of least-privilege design is that an admin changing a server IP address should not be able to read the database password sitting in the same file, let alone execute arbitrary shell commands as the web server user. This bug turns a narrow, auditable action into total compromise. That gap between what an action is supposed to do and what it actually allows is usually where the real severity of a bug lives, and it's the question I try to ask about every finding now, not just this one.
What actually fixed it
The FOSSBilling team's patch was small and correct: swap the hand-rolled string template for PHP's own var_export(), which escapes correctly by construction.
// After
return ' => ' . var_export($value, true) . ',' . PHP_EOL;I reviewed the patched commit on the private advisory fork before it shipped and confirmed the same payload comes out safely escaped. That's a step I try never to skip: a patch that "looks right" and a patch I've actually re-tested against my own payload are not the same thing.