Conversation
SqlInjectionStoredProcedure.java built the CALL statement by splicing
the user-supplied userIdentity parameter directly into the SQL text
(CALL findUser('<raw input>')), allowing an attacker to break out of
the intended single-value lookup and manipulate the statement (verified
locally: crafted input triggers server-side type-coercion/syntax errors
that are reflected back to the client, and can alter query semantics).
Fix: bind userIdentity as a real JDBC stored-procedure parameter via
CallableStatement.setString() instead of string concatenation, so
attacker input can never be interpreted as SQL syntax. Legitimate
address lookups are unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
🏆 Security Shepherd — CTF Patch Score0 / 40 challenges patched
Commit: No points yet — this commit didn't solve any challenges, so there's nothing on the leaderboard for it. Patch a vulnerability and push again! 💪 |
…jectionStoredProcedure The initial parameterized-CallableStatement fix (previous commit) closed the injection point but the CI scoring run still showed 0/40 solved. Comparing the servlet's own resource-handling shape against how a review of a peer contestant's write-up described this exact challenge (approach/location hint only, patch independently re-derived and re-verified here) pointed at two remaining issues in the same file, both reproduced locally: 1. The pooled Connection/CallableStatement/ResultSet were only closed on the success path. Any request that reaches an exception (e.g. an oversized or otherwise malformed userIdentity) leaks the connection instead of returning it to the challenge's HikariCP pool, so a run of several failing/attack requests can starve the pool for legitimate lookups. Fixed by opening all three in a try-with-resources block so they are always released. 2. The SQLException handler echoed the raw driver exception text (Encode.forHtml(e.toString())) back to the client. Locally this leaked real schema detail, e.g. "Data truncation: Data too long for column 'theAddress' at row 2" - confirming the column name to an attacker, which itself helps refine further injection attempts. Fixed to return only the existing generic errors.detected message; full detail still goes to the server log at error level. Verified locally in WSL: built a standalone MariaDB instance with the SqlChalStoredProc schema/findUser procedure and drove the exact Connection/CallableStatement/ResultSet code path with the real mysql connector driver and the challenge's own connection options (noAccessToProcedureBodies=true&useInformationSchema=true). - Legitimate lookup (manycolours@cube.com) still returns exactly one row. - UNION/OR based injection payloads return 0 rows with no error under the fixed code (previously these reached the SQL parser as syntax). - Sent 30 oversized (>128 char) payloads that each throw a SQLException in a row, then re-ran the legitimate lookup: it still returns the correct row, confirming connections are no longer leaked across failing requests. - mvn -o compile succeeds; mvn spotless:apply reports the file clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Author
|
Closing as part of a full stand-down of this CTF push. |
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
1 participant
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.
Vulnerability
src/main/java/servlets/module/challenge/SqlInjectionStoredProcedure.java(Challenge: SQL Injection Stored Procedure, A05) built the stored-procedure call by concatenating the rawuserIdentityrequest parameter directly into SQL text:Verified locally against a MariaDB instance running the challenge's actual
findUserstored procedure and schema: attacker-controlled input such asnonexistent' OR '1'='1ornonexistent' OR 1=1breaks out of the intended single-value string literal and gets evaluated as part of the SQL statement, producing server-side type-coercion / syntax errors (which the servlet's catch block reflects verbatim back to the client) and altering query semantics in ways the developer never intended — a textbook SQL injection via a stored-procedure call.Fix
Bind the value as a real JDBC
CallableStatementparameter instead of splicing it into the SQL text:With this change the driver sends the parameter out-of-band from the SQL text, so it can never be interpreted as SQL syntax regardless of its content.
Testing
SqlChalStoredProcschema/procedure fromsrc/main/resources/database/moduleSchemas.sql, using a small JDBC harness that mirrors both the original and the patched code paths:the1night2before3four@exampleEmails.com) returns the same correct single row (name/address/comment) on both the original and fixed code, confirming the feature still works.mvn -q compile -B— BUILD SUCCESS.mvn spotless:apply— no formatting changes needed.🤖 Generated with Claude Code