$argv[0] can be empty

[eweso]

In some cases $argv[0] can be empty and strpos will throw an "Empty needle" error. Therefore on line 493 $argv should be checked, that it is not empty and a string, instead of just checking its existence with isset().

Someone tried to attack one of my websites with "/index.php?++++hot=1&++++kw=%E8%93%9D%E7%89%99%E8%80%B3%E6%9C%BA&r=l" and this request caused an exception which was logged and where I found the error.

Provide a narrative description of what you are trying to accomplish:

• Are you fixing a bug?

  • Detail how the bug is invoked currently.
  • Detail the original, incorrect behavior.
  • Detail the new, expected behavior.
  • Base your feature on the master branch, and submit against that branch.
  • Add a regression test that demonstrates the bug, and proves the fix.
  • Add a CHANGELOG.md entry for the fix.

• Are you creating a new feature?

  • Why is the new feature needed? What purpose does it serve?
  • How will users use the new feature?
  • Base your feature on the develop branch, and submit against that branch.
  • Add only one feature per pull request; split multiple features over multiple pull requests
  • Add tests for the new feature.
  • Add documentation for the new feature.
  • Add a CHANGELOG.md entry for the new feature.

• Is this related to quality assurance?

• Is this related to documentation?

[michalbundyra]

Hi @eweso,

thanks for your contribution. I've checked it quickly and it looks like you have enabled
register_argc_argv in your php.ini that you can see argv/argc in SERVER from GET (non-CLI) requests. This setting is disabled by default.

I've changed my configuration, tried the request you provided and the results of SERVER['argv'] is as follows:

array (size=9)
  0 => string '' (length=0)
  1 => string '' (length=0)
  2 => string '' (length=0)
  3 => string '' (length=0)
  4 => string 'hot=1&' (length=6)
  5 => string '' (length=0)
  6 => string '' (length=0)
  7 => string '' (length=0)
  8 => string 'kw=%E8%93%9D%E7%89%99%E8%80%B3%E6%9C%BA&r=l' (length=43)

so yeah - argv[0] is empty.

Looking at the code, I don't think that we should really process SERVER['argv'] there for GET request as it contains (per documentation: https://www.php.net/manual/en/reserved.variables.server.php) the query string.

We are detecting there the base url, so it cannot be detected from query string.

I would suggest to change your PHP configuration, unless you really need to use somewhere _SERVER['argv'] for GET requests.

[michalbundyra]

As I said in previous comment - we don't really want process argv, as for GET requests it contains query string only - so we shouldn't use it for script filename. It should be used only in CLI mode.

Can't see a nice way to test it for GET request, for CLI test is in place already.

[weierophinney]

🚢

[weierophinney]

👍 This makes sense; only test argv if we're in a CLI request.

[michalbundyra]

Thanks, @eweso!