Skip to content

Commit

Permalink
fix possible vulnerability using mongolite
Browse files Browse the repository at this point in the history
  • Loading branch information
@aheinze
aheinze committed Mar 17, 2021
1 parent 4923371 commit b40d6bd
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions lib/MongoLite/Database.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,10 @@ public static function buildCondition($criteria, $concat = ' && ') {

$d = '$document';

if (\strpos($key, '(') !== false) {
throw new \InvalidArgumentException('Unallowed characters used in filter keys');
}

if (\strpos($key, '.') !== false) {

$keys = \explode('.', $key);
Expand Down

3 comments on commit b40d6bd

@abernh
Copy link
Contributor

@abernh abernh commented on b40d6bd Jul 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the researchers vulnerability article (https://swarm.ptsecurity.com/rce-cockpit-cms/)
And this seems not to fix this particular issue completely:

So by controlling the content of the $key variable, we can escape from the string literal (break it) with a single quote in order to inject arbitrary PHP code.

Making sure that the key string does not contain any parenthesis will prevent it from executing most functions.
But there are still some functions that are invokable without parenthesis

echo()
include()
include_once()
require()
require_once()
return()
print()

Can you explain why you are not checking for single quotes instead?

@aheinze
Copy link
Member Author

@aheinze aheinze commented on b40d6bd Jul 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read the researchers vulnerability article (https://swarm.ptsecurity.com/rce-cockpit-cms/)
And this seems not to fix this particular issue completely:

So by controlling the content of the $key variable, we can escape from the string literal (break it) with a single quote in order to inject arbitrary PHP code.

Making sure that the key string does not contain any parenthesis will prevent it from executing most functions.
But there are still some functions that are invokable without parenthesis

echo()
include()
include_once()
require()
require_once()
return()
print()

Can you explain why you are not checking for single quotes instead?

Hi Andreas 👋

Good point! Would you mind creating a pull request?

@abernh
Copy link
Contributor

@abernh abernh commented on b40d6bd Jul 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. #1457

Please sign in to comment.