A few days ago, I was working on an internal player API for SiegeGG Pro and encoutered this error:

Uncaught ArgumentCountError: exec() expects at least 1 argument, 0 given

Now, here is the thing, I never called that function. The exec function in PHP executes an external program. At SiegeGG, we only use the function for one feature: starting our data importer. However, I was working with player data. That data is already imported and there is no reason why the importer would be triggered.

So, if it’s not because of the importer, how did the exec function end up getting called? Let’s dig into it.

The error was thrown in the following line of codingpaws/expose, our dependency we use to simplify the creation of API entities:

$value = is_callable($value) ? $value() : $value;

This line of code gets executed to generate the value of an API field. For example, the kill-death ratio of a player will result in a division by zero if there are no deaths. In that case, we could conditionally expose the API field and dynamically generate the value.

In codingpaws/expose syntax, that instruction would look like this:

$this->expose(
    'kd',
    fn () => $player->kills / $player->deaths,
    if: $player->deaths > 0
);

The second argument becomes the $value in the previous code example. If the $value is a callback, and therefore callable, we want to call it to get the value it returns if the player has died more than zero times.

But, how did this end up calling the exec function? Remember, the importer is detached from the API and not active.

To find an answer, I narrowed down the error to the Rainbow Six Siege esports player Exec, who last played 6 years ago. And you might have a suspicion why this caused the error. It turns out, in PHP, a string is callable if a global function exists that is named like the string value.

Therefore, the is_callable($value) is true for Exec’s player name as a function with the same name exists, it’s—surprise—exec. Expose goes ahead and calls exec for us. This is technically correct but very, very bad here. Exec is, semantically, a player name and not a function. But in PHP, exec is also the function.

This unintuitive behavior of PHP can even be a security vulnerability as, right here, Expose deals with arbitrary user data. While an external user can’t control the arguments, in some cases, they could execute any global function. As the resulting $value will be sent via the API to the requesting user, getting the result of a global Laravel function like request or app could potentially expose secrets and lead to a compromise of the system or user data.

Luckily, the fix for this is simple. We just need to check that the $value is not a string and callable to call it. I fixed it in 3b05e9f2 (and previously hotfixed it in SiegeGG production when I discovered the bug).

-$value = is_callable($value) ? $value() : $value;
+$value = !is_string($value) && is_callable($value) ? $value() : $value;

The design decision allowing strings to be callable might’ve been a useful change when it was made years ago. But it introduces the risk that arbitrary data becomes callable, potentially opening an application up for data leaks or other damage. Unfortunately, this is not explained in the is_callable docs, although it is somewhat implied.

Thanks to the introduction of the First-class Callable Syntax, we technically no longer need is_callable to be true for names of global functions. But it’s still in PHP for backwards compatibility and will probably stick with us until at least PHP 9.0, the next major PHP version.