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.

Sign up for GitHub

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

Jump to bottom

Add Admin_Resource on Admin controllers + Add escaper on templates + Add bind on fetchRow to avoid risk of SQL Injection #49

Open

bretin wants to merge 1 commit into Riskified:master from bretin:security-escaper-acl-injection

bretin commented Jan 6, 2023

During a technical assessment, I noticed a few security issues:

The admin controllers have no admin_resource, we added sales_order ACL since it's on the order grid but it could be a custom ACL
A direct SQL query has been adjust to use a binding variable instead of of the direct use in order to avoid risk of SQL injection
Template has been adjust to use escapers


          Add Admin_Resource on Admin controllers + Add escaper on templates + …

326956e

…Add bind on fetchRow to avoid risk of SQL Injection

Contributor

Daniel-Shai commented Jan 18, 2023

Hi @wkaminsk , can you please take a look? It's a suggestion from SAQ

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

Labels

None yet