False Negative with https://github.com/robmoffat/codeql-vuln-blog

[robmoffat]

Description of the issue

I forked the repo: https://github.com/robmoffat/codeql-vuln-blog

and then added the CodeQL GH action, but no vulnerabilities are reported.

Reviewing the python code in the repo, it seems ripe with SQL injection possibilities.

What am I doing wrong?

thanks

[robmoffat]

@maoo @theJuanAndOnly99

[tausbn]

Thank you for your report! While investigating this, I discovered an issue that was preventing us from identifying the sqlite3.execute calls as sinks for the SQL injection query.

However, even though we now identify these as sinks, the query still has no results. I would therefore like to understand the SQL injection possibilities you refer to a bit better.

Currently, we only consider the first call of the execute method (of a module that implements PEP249) to be vulnerable. This is the argument that contains the SQL query itself (and so if an attacker can control this, then it's game over).

We do not, however, consider the values that are interpolated into this string (by passing them as the second argument) to be problematic.

So for instance in the code

@bp.route('/login', methods=('GET', 'POST'))
def login():
    if request.method == 'GET':
        return render_template('auth/login.html')

    username = request.form['username']
    password = request.form['password']
    db = get_db()
    error = None
    user = db.execute(
        'SELECT * FROM user WHERE username = ?', (username,)
    ).fetchone()

we identify username as being user-controlled, but do not consider the use of it in the penultimate line to be problematic (which to my knowledge is the right interpretation).

Can you point me to one of the SQL injections?

[robmoffat]

thanks for your prompt reply! Let me work on this a bit...

[robmoffat]

I added this to the codebase:

@bp.route('/<int:post_id>/something/<title>', methods=('GET', 'POST'))
def vuln(post_id, title):
    db = get_db()
    db.execute( 'UPDATE post SET title = "'+title+'" WHERE id = '+post_id)
    return redirect(url_for('blog.index'))

Seems like a clear opportunity for an SQL injection attack. No issues reported by CodeQL. Did I miss something?

https://github.com/robmoffat/codeql-vuln-blog/runs/6423833444?check_suite_focus=true

thanks, Rob

[robmoffat]

any thoughts? thanks for your time.

[tausbn]

I agree, this does look like something that should be caught by our analysis. I'll investigate why it isn't getting picked up.