Skip to content
This repository was archived by the owner on Nov 4, 2024. It is now read-only.

Conversation

@rolfen
Copy link

@rolfen rolfen commented Oct 20, 2018

This is a fix for this issue:
#1790

It fixes the SQL injection vulnerability that I found in this plugin.

I rewrote the database code to make use of PDO prepared statement and bound variables instead of str_replace().
I also improved the description in the plugin config.

I tried to respect backwards compatibility with the current version of the plugin, so that current users of the plugin won't need to re-write their custom queries.

However it could conceivable fail for a few edge cases. For example, PDO prepared statements can't have dynamic column names such as SELECT :domain .... But who would want this?

@RainLoop RainLoop merged commit 495fb48 into RainLoop:master Oct 20, 2018
@rolfen
Copy link
Author

rolfen commented Oct 21, 2018

Thank you for merging.

Worrying about backwards compatibility might have been premature - this plugin does not seem to be widely used and and if the user followed instructions, it would be copied, not linked to the data directory, so not affected by the update.

I can do a PR for a slightly improved and leaner version without the backwards compatibility and without the "table" input - it may be better to write the table name directly in the custom query. Please let me know if interested.

Good job on Rainloop.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants