Skip to content
This repository has been archived by the owner on Jul 17, 2020. It is now read-only.

Problems with escaping #1

Open
bolner opened this issue Apr 4, 2020 · 3 comments
Open

Problems with escaping #1

bolner opened this issue Apr 4, 2020 · 3 comments

Comments

@bolner
Copy link

bolner commented Apr 4, 2020

Hello,

there are some bugs in the mapi_quote function, which is responsible for escaping. Practically that function does nothing currently.

https://github.com/MonetDB/monetdb-php/blob/92435e835410c2ae088830c31583e4f3b5519188/lib/php_mapi.inc#L796

Nearly all single quotes need to be replaced by double quotes. Just execute this in a console to see:

$ php -a
php > echo '\"';
\"
php > echo '\n';
\n
php >

Then check the same with double quotes:

$ php -a
php > echo "\"";
"
php > echo "\n";

php >

Etc. As you can see the single quote disables most of the escaping, except some, like '\'' and '\\'. So the code compares 2-character strings to single characters.

Also, some characters are missing. For example the % wildcard character or the '\r' carriage return character.

A proper implementation would have at least 3 requirements:

  • Since MonetDB always uses UTF-8, the PHP code only has to check if the multi-byte support is enabled or not, and if the current character encoding is UTF-8. If any of these two requirements were false, then it has to throw an error, as proper escaping is not possible. (There are many known sql-insertion attacks use the differences in character sets and the translations between them.)
  • MonetDB needs to a have support for unicode escape sequences in its strings. (I found nothing about this online.)
  • The escaping function has to be implemented in full knowledge of the query parsing code of MonetDB. (Just think of MySQL/PHP for which it took 10 years to implement proper escaping.)

But that best would be to use query parameters (or prepared statements) and pass the parameters independently, outside of the query.

@kutsurak
Copy link
Member

kutsurak commented Apr 9, 2020

Hello,

Thanks for the reports. For a number of reasons it will take some time to go through them. If you prepare patches we will try to review them in a timely manner.

As a side note this repository is a mirror, that is we do not use it for development. The source is maintained at https://dev.monetdb.org/hg/monetdb-php/

@bolner
Copy link
Author

bolner commented Apr 10, 2020

@kutsurak Hi, thanks for the response.
I've created another client library in the meantime:
https://github.com/bolner/MonetDB-PHP-Deux

best,
Tamas

@kutsurak
Copy link
Member

Hey @bolner,

This is great. From a (very) brief look, and from the perspective of someone who doesn't know a lot of php, this looks good! Thank you very much for your effort.

best regards,
Panos.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants