Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.16] disallow preparing statements that look like SELECT * FROM ... #110

Open
jhgg opened this issue Jan 21, 2021 · 3 comments
Open

[0.16] disallow preparing statements that look like SELECT * FROM ... #110

jhgg opened this issue Jan 21, 2021 · 3 comments
Labels
enhancement help wanted improved-api Work towards a 1.0 release (initially 0.18)

Comments

@jhgg
Copy link
Collaborator

jhgg commented Jan 21, 2021

A prepared statement that has a SELECT * in it is unsafe.

More details here:

This is a very very bad footgun, and can cause your driver to return incorrect rows when a table is altered.

We should disallow these kind of queries from being prepared, as it can introduce soundness bugs in one's code.

@kw217
Copy link
Collaborator

kw217 commented Jan 22, 2021

Ouch! Yes, that's an unpleasant one indeed. It's a pity that Cassandra still doesn't protect us from this.

However, is there a sensible way to spot this without fully parsing the query? I guess just checking for statements starting with /SELECT +\*/i would go a long way.

@jhgg
Copy link
Collaborator Author

jhgg commented Jan 23, 2021

Our Python wrapper for cassandra has the following check when preparing statements, which seems to work good enough!

def assert_safe_prepared_statement(query_string: str) -> None:
    query_tokens = [p.strip() for p in query_string.lower().strip().split()]
    if len(query_tokens) >= 2 and query_tokens[0] == 'select' and query_tokens[1] == '*':
        raise ValueError('Cannot prepare `select *` like statement. The query_string was %r' % query_string)

I think we can make the Rust one a bit more idiomatic with this:

fn is_unsafe_prepared_statement(query: &str) -> bool {
    let mut tokens = query.split_ascii_whitespace();
    match tokens.next() {
        Some(tok) if tok.to_ascii_lowercase() == "select" => { }
        _ => return false,
    }
    
    match tokens.next() {
        Some(tok) if tok == "*" => true,
        _ => false
    }
}

@kw217
Copy link
Collaborator

kw217 commented Feb 2, 2021

Yep, looks good to me - it's best efforts but it will catch most badness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted improved-api Work towards a 1.0 release (initially 0.18)
Projects
None yet
Development

No branches or pull requests

2 participants