Skip to content

Conversation

@samialfattani
Copy link
Contributor

A run-time error is raised when get_one() is called in flask_admin.contrib.sqla.view.ModelView

To replicate the error try to browse this: /admin/details/?id=1,2

Risk:
A security vulnerability is raised by Tnable where it could lead to Blind SQL Inejection which is classified as a High risk vulnerability. This can be exploited in /edit and /details

image

A test case is added to make sure that /admin/details/?id=1,2 is working as expected

@samialfattani samialfattani marked this pull request as ready for review February 8, 2026 12:00
@ElLorans
Copy link
Contributor

ElLorans commented Feb 8, 2026

The value is passed to SQLAlchemy’s Session.get() using bound parameters, not interpolated SQL, so the security vulnerability is a false positive.
Passing 1,2 on a model with int primary key returns a 400 response, which is correct, so I am inclined towards closing this PR.

if isinstance(self._primary_key, tuple):
_id = tools.iterdecode(id)
else:
_id = (tools.escape(id),)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why tools.escape?
Looks unnecessary to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, what if the actual string-single-PK="1,2", is it possible to navigate /admin/details?id=1,2 without escaping ?

@samialfattani
Copy link
Contributor Author

The value is passed to SQLAlchemy’s Session.get() using bound parameters, not interpolated SQL, so the security vulnerability is a false positive. Passing 1,2 on a model with int primary key returns a 400 response, which is correct, so I am inclined towards closing this PR.

Actually it returns 500!, here is what we got without this PR

image

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants