How to allow SELECT queries and prevent others?
Asked Answered
C

8

19

In our application, users can create custom export functions in form of SQL statements. Something like this:

SELECT name, age, date_birth FROM users WHERE group_id = 2

I don't want them to clear the whole database by inserting a DELETE statement. My ideas would be:

  • Use an SQL account, which is only allowed to SELECT. (I don't want to do this, if there are alternatives.)
  • Use a magic regex, that checks whether the query is dangerous or not. (Would this be good? Is there already such a regex?)

We are using PHP PDO.

Censorship answered 7/5, 2015 at 8:34 Comment(5)
It's very bad practice if you allow the user to execute SQL queries on your server. I recommend you to use some REST api to allow interaction with your database.Hydrodynamics
I think regex would be ok, just check if there are words like 'delete', 'update' and or 'insert' and 'drop' and many moreHacker
but the best answer to this is to create a program where they can choose/click/select columns and other stuffs in a graphical way, and then build the query string at the backendHacker
Best option would be to have a user that is only allowed to do SELECT queries. Create a seperate user for your INSERT, UPDATE and DELETE queries.Stomy
Only the admin will have access to this. The problem is, that queries can be complex and such a click interface would be a huge effort (joins, concat, inline SELECT statements..). Is there a PHP library that can do just that?Evenings
S
24

As I see it, there are three options to choose from:

Option 1

Create a tool that will create the query for the user on the background. Simply by clicking buttons and entering table names. This way you can catch all weird behavior in the background bringing you out of danger for queries you don't want executed.

Option 2

Create a MySQL user that is only allowed to do SELECT queries. I believe you can even decide what tables that user is allowed to select from. Use that user to execute the queries the user enters. Create a seperate user that has the permissions you want it to to do your UPDATE, INSERT and DELETE queries.

Option 3

Before the query is executed, make sure there is nothing harmfull in it. Scan the query for bad syntax.

Example:

// Check if SELECT is in the query
if (preg_match('/SELECT/', strtoupper($query)) != 0) {
    // Array with forbidden query parts
    $disAllow = array(
        'INSERT',
        'UPDATE',
        'DELETE',
        'RENAME',
        'DROP',
        'CREATE',
        'TRUNCATE',
        'ALTER',
        'COMMIT',
        'ROLLBACK',
        'MERGE',
        'CALL',
        'EXPLAIN',
        'LOCK',
        'GRANT',
        'REVOKE',
        'SAVEPOINT',
        'TRANSACTION',
        'SET',
    );

    // Convert array to pipe-seperated string
    // strings are appended and prepended with \b
    $disAllow = implode('|',
        array_map(function ($value) {
            return '\b' . $value . '\b';
        }
    ), $disAllow);

    // Check if no other harmfull statements exist
    if (preg_match('/('.$disAllow.')/gai', $query) == 0) {
        // Execute query
    }
}

Note: You could add some PHP code to filter out comments before doing this check

Conclusion

What you are looking to do is quite possible however you'll never have a 100 percent guarantee that it's safe. Instead of letting the users make the queries it's better to use an API to provide data to your users.

Stomy answered 7/5, 2015 at 9:3 Comment(3)
What if a column name is called delete_id for example? It will not run? Maybe an even safer way would be to do something like ' DELETE ', with spaces on both sides to really make sure to match the word?Chronic
@JensTörnell You've got a good point there, though if we use ` DELETE , queries starting with DELETE` wouldn't be matched. I'll give it some more thought.Stomy
@JensTörnell I have made some slight changes so that delete_id will not be matched with DELETE. The check is now case-insensitive as well.Stomy
L
12

Don't do this, there will always be creative ways to make a dangerous query. Create an API that will manually construct your queries.

Lamarlamarck answered 7/5, 2015 at 8:40 Comment(0)
A
6

Since you said you would prefer not to use read only SQL accounts if there are alternatives. If you're running PHP 5.5.21+ or 5.6.5+: I'd suggest checking if the first statement of the query is a SELECT statement and disabling multiple queries in your PDO connection.

First disable multi statements on your PDO object...

$pdo = new PDO('mysql:host=hostname;dbname=database', 'user', 'password', [PDO::MYSQL_ATTR_MULTI_STATEMENTS => false]);

Then check that the first statement in the query uses SELECT and that there are no subqueries. The first regex ignores leading whitespace which is optional and the second detects parenthesis which would be used to create subqueries -- this does have the side effect of preventing users from using SQL functions but based on your example I don't think that's an issue.

if (preg_match('/^(\s+)?SELECT/i', $query) && preg_match('/[()]+/', $query) === 0) {
    // run query
}

If you're running an older version, you can disable emulated prepares to prevent multiple statements being executed but this relies on PDO::prepare() being used.

FWIW: It would be a lot better to use prepared statements/generate safe queries for your users or to use a read-only SQL account. If you're using MySQL and have admin rights/remote access, I'd suggest using the community edition of SQLyog (https://github.com/webyog/sqlyog-community/wiki/Downloads) to create read-only user accounts. It's extremely user friendly so you won't have to learn the GRANT syntax.

Almaalmaata answered 10/12, 2015 at 20:55 Comment(0)
R
3

For me,I'm prefer to use a MYSQL account only allowed SELECT.

If you want a regex,I think all SELECT SQLs are started with "select",any others? These are my codes:

$regex = "/^select/i";
if(preg_match($regex,$sql)){
    //do your sql
}
Recollected answered 9/12, 2015 at 10:51 Comment(0)
Z
3

I wanted a safer, more robust solution that didn't involve fully tokenizing query. Based on my experience writing SQL parsers (here, and here), I can say this solution is pretty bulletproof without having to use a full-featured query parser.

This is the best answer because:

  • It does not require a new SQL user with limited permissions
  • It does not require a new DB connection with limited permissions
  • It does not break if the query contains a string or a comment with the word "delete"
  • It allows complex queries with nested queries
  • It allows the user to input arbitrary SQL
  • It's safe

All the other answers as of the time of writing this have one or more of these limitations.

Here's how it works

  • Remove all inline and multi-line comments
  • Remove all single and double quoted strings
  • Remove all symbols and numbers
  • Create a unique array of the remaining (key)words
  • If any of the remaining keywords are INSERT, UPDATE, DELETE, RENAME, DROP, CREATE, TRUNCATE, ALTER, COMMIT, ROLLBACK, MERGE, CALL, EXPLAIN, LOCK, GRANT, REVOKE, SAVEPOINT, TRANSACTION, or SET then it is a "dangerous" query and should not be run.

Note:

This function does not validate the SQL, it only ensures that the SQL will not alter your database in any way. You will still need to run the query in a try/catch to make sure the query is valid.

/**
 * Determine if an SQL statement could potentially alter the database in any way.
 * @param string $sql - An SQL statement
 * @return boolean - True if query could alter the database, else false
 */
function isDangerousQuery($sql){
    $sql = trim($sql);

    // Irrelevant tokens to be parsed out of the query
    // A comment or string may contain a word like "drop"
    // so comments and strings need to be removed from the query
    $token_types = [
        [   'name' => 'Single-Line Comment',
            'start' => "--",
            'end' => "\n"   ],
        [   'name' => 'Multi-Line Comment',
            'start' => "/*",
            'end' => "*/"   ],
        [   'name' => 'Double-quoted String',
            'start' => "\"",
            'end' => "\""   ],
        [   'name' => 'Single-quoted String',
            'start' => "'",
            'end' => "'"    ]
    ];
    
    // This array will contain every character that is not part
    // of one of the above described irrelevant tokens
    $keywords_buffer = [];
    
    // If we are currently parsing one of the above token types
    // it's index is held here, else this will be false
    $current_token_type_index = false;
    
    // Loop through each character and reconstruct the query without the 
    // irrelevant token types. We need to loop rather than use a regex
    // because there could be quotes nested in comments and things like that
    // that would "trick" our regex
    $length = strlen($sql);
    for ($index = 0; $index < $length; $index++) {
        $chunk = substr($sql, $index);

        // If the current char is an escape char, skip the next char
        if($sql[$index] === '\\'){
            $index++;
            continue;
        }

        // Looking for all starting tokens
        if(false === $current_token_type_index){

            foreach($token_types as $token_type_index => $token_type){
                if(0 === strpos($chunk, $token_type['start'])){
                    $current_token_type_index = $token_type_index;
                }
            }

            if(false === $current_token_type_index){
                $keywords_buffer[] = $sql[$index];
            }

        // Looking for ending token
        }else if(0 === strpos($chunk, $token_types[$current_token_type_index]['end'])){

            $index += strlen($token_types[$current_token_type_index]['end']);
            if(strpos($token_types[$current_token_type_index]['end'], "\n") !== false) $keywords_buffer[] = "\n";
            $current_token_type_index = false;

        }

    }
    
    // Reconstruct the sql without the irrelevant tokens
    $sql_cleaned = implode('', $keywords_buffer);
    
    // Remove all symbols from the sql leaving only keywords and numbers
    $sql_keywords_only = preg_replace("/[^a-zA-Z_0-9\s]/", ' ', $sql_cleaned);
    
    // Create an array of unique keywords in upper-case
    $sql_keywords = array_unique(preg_split("/\s+/", strtoupper($sql_keywords_only)));
    
    // Filter out numbers and empty strings to get actual keywords
    $sql_keywords_filtered = [];
    foreach($sql_keywords as $keyword){
        if(!empty($keyword) && !is_numeric($keyword)){
            $sql_keywords_filtered[] = $keyword;
        }
    }
    
    // list of forbidden/dangerous keywords
    $dangerous_keywords = [
        'INSERT',
        'UPDATE',
        'DELETE',
        'RENAME',
        'DROP',
        'CREATE',
        'TRUNCATE',
        'ALTER',
        'COMMIT',
        'ROLLBACK',
        'MERGE',
        'CALL',
        'EXPLAIN',
        'LOCK',
        'GRANT',
        'REVOKE',
        'SAVEPOINT',
        'TRANSACTION',
        'SET'
    ];
    
    // Contains an array of dangerous keywords found
    // If this array is empty, query is safe
    $found_dangerous_keywords = array_intersect($dangerous_keywords, $sql_keywords_filtered);
    
    return count($found_dangerous_keywords) > 0;
}
Zach answered 4/6, 2021 at 15:59 Comment(1)
You also want to add in "REPLACE" as a dangerous keyword.Immunize
C
2

You can check whether there is any DELETE statement in the query by the following code.

if (preg_match('/(DELETE|DROP|TRUNCATE)/',strtoupper($query)) == 0){
    /*** Run your query here ***/
}
else {
    /*** Do something ***/
}

Note as pointed out by peter it is better to have a seperate MySql user.

Convenient answered 4/12, 2015 at 18:41 Comment(0)
M
0

There is another method, if you are using the C API, using the mysql_stmt_prepare() method will allow you to query the field_count, which will be non-zero on a select statement.

It also allows you to use proper sql preparation instead.

Malignant answered 16/4, 2019 at 11:20 Comment(0)
S
0

I am doing this using python and its work proper

import core
import os
import sqlparse
from sqlalchemy import create_engine, text
from sqlparse.sql import Token, TokenList
from sqlparse.tokens import Keyword
from typing import Iterable
from collections import deque


passes_blacklist_flag, failing_words = is_select_query(query)
if passes_blacklist_flag is False:
    return {"error":"Only SELECT queries are permitted. "+ ", ".join(failing_words)+ " queries are not allowed."}


def is_select_query(query):
    sql_strings = sqlparse.split(query)
    keyword_tokens = set()
    for sql_string in sql_strings:
        statements = sqlparse.parse(sql_string)
        for statement in statements:
            for token in walk_tokens(statement):
                if not token.is_whitespace and not isinstance(token, TokenList):
                    if token.ttype in Keyword:
                        keyword_tokens.add(str(token.value).upper())
                        
    SQL_BLACKLIST = ['INSERT', 'UPDATE', 'DELETE', 'CREATE', 'RENAME', 'DROP', 'ALTER', 'COMMIT', 'TRUNCATE', 'EXPLAIN', 'LOCK', 'GRANT', 'REVOKE', 'SAVEPOINT', 'TRANSACTION', 'SET', 'ROLLBACK', 'EXEC', 'MERGE', 'CALL', 'REPLACE']
    fails = [bl_word for bl_word in SQL_BLACKLIST if bl_word.upper() in keyword_tokens]
    
    return len(fails) == 0, fails
    
   
def walk_tokens(token: TokenList) -> Iterable[Token]:
    queue = deque([token])
    while queue:
        token = queue.popleft()
        if isinstance(token, TokenList):
            queue.extend(token)
        yield token
Sclerosed answered 18/7, 2024 at 13:1 Comment(1)
As it’s currently written, your answer is unclear. Please edit to add additional details that will help others understand how this addresses the question asked. You can find more information on how to write good answers in the help center.Caiman

© 2022 - 2025 — McMap. All rights reserved.