Prevent SQL Injection in AdminAbstractController - #7
Open
pixeebot-analysis-service[bot] wants to merge 1 commit into
Open
pixeebot-analysis-service[bot] wants to merge 1 commit into
pixeebot-analysis-service[bot] wants to merge 1 commit into
Conversation
|
New Issues (27)
Checkmarx found the following issues in this Pull Request
Fixed Issues (38)
Great job! The following issues were fixed in this Pull Request
Communicate with Checkmarx by submitting a PR comment with @Checkmarx followed by one of the supported commands. Learn about the supported commands here. |
|
@pixeebot testing SCM PR feedback capture end-to-end. This looks like a good fix! |
|
@pixeebot second attempt testing SCM PR feedback capture end-to-end. This looks like a good fix! |
|
@pixeebot third attempt testing SCM PR feedback capture end-to-end. This looks like a good fix! |
|
@pixeebot fourth attempt testing SCM PR feedback capture end-to-end. This looks like a good fix! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pixee Scan: 6bcf5ede-043b-4d69-b13d-2d9bafab1c12
Confidence: HIGH
Fix confidence is a rating derived from an internal benchmark and includes High, Medium, and Low confidence fixes. It comprises three weighted scores reflecting the safety, effectiveness and cleanliness of Pixee's code changes within a fix. View Details in Pixee.
Remediation
This change fixes finding 320caa37-ccf5-4d5d-84c6-985c98827a4e.
Details
Prevent SQL Injection in AdminAbstractController
Summary
Validated admin sort-property handling in
AdminAbstractControllerto keep untrusted identifiers out of query construction.Vulnerability Description
SQL Injection occurs when an application builds a database or Java Persistence query with attacker-controlled input. If user-supplied data reaches query text without strict validation or parameterization, an attacker can change the query structure instead of just its values. That can expose sensitive records, bypass authorization checks, or modify and delete data.
Changes Made
AdminAbstractController.javapreviously copied request-provided sort property names directly intoFilterAndSortCriteria, which allowed untrusted identifier data to flow into later query building. Because those sort fields are used to constructORDER BYclauses, the pattern was dangerous even though the values themselves were not being parameterized. The fix introduced a validation check for each sort property before it is used. Invalid values now trigger anIllegalArgumentException, and the controller only continues with validated property names when it populates or updates the criteria map. This keeps the query-building path constrained to safe sort identifiers at the admin controller boundary.Guidance Adherence
Source: Pixee Knowledge Base
Applied the following guidance from the remediation instructions:
isValidSortProperty(...)validation inAdminAbstractController.getCriteria(...)and does not modify any SQL/JPA query construction, prepared statement, or query call site as required by the guidance.^[a-zA-Z0-9_]+(?:\\.[a-zA-Z0-9_]+)*$) forsortProperty, which matches the guidance’s validation approach for dynamic identifiers. However, the vulnerable flow provided is not a raw SQL identifier-construction issue; it is a data flow intoFilterAndSortCriteria/query-building infrastructure, and the fix does not demonstrate the required parameterized-query remediation at the actual sink.Because the patch adds input validation in controller sorting logic but does not address the actual SQL injection remediation pattern described in the guidance, I am rejecting it as not aligned with the expected fix.