fix: close JDBC resources in Session Management 6 and 7 challenges (part of #857) by ismisepaul · Pull Request #1005 · OWASP/SecurityShepherd

ismisepaul · 2026-10-25T00:59:47Z

⚠️ Manual testing required before merging

The automated tests prove these levels no longer exhaust their connection pools. They don't prove each level can still be solved the intended way, and these are deliberately vulnerable training levels.

Please run this branch, follow the steps, and comment on this PR with the result: pass, or the step that failed and what you saw.

Branch: dev#857-session-management-6-7

Set up. Use a machine where the Shepherd Docker volumes can be thrown away: down -v deletes their data. JDK 17 and Maven 3.9.2+ are needed.

git fetch origin
git checkout 'dev#857-session-management-6-7'
mvn -Pdocker clean install -DskipTests
docker compose down -v
docker compose up -d --build

Open https://localhost, accept the self-signed certificate warning, log in as admin / password and set a new password when asked.
On the Get Started page, tick Enable Unsafe Levels and click Open All Levels.
Admin → Module Management → Change Module Layout → Open Floor, then reload the page, so every level is listed on the left.
Admin → Configuration → Cheat Sheet Management → Enable Cheat Sheets for Administrators. With a level open, the Cheat button in the left-hand menu then shows its solution.

Test

Solve Session Management Challenge 6 and 7 with the solutions from their Cheat buttons and check you get the result keys.
In Session Management Challenge 6, look up the secret question with a broken email such as ' five times in a row. Each should show the SQL error, as before. Then look up a valid address: it should show its question straight away.
In both levels, try a wrong login five times, then a wrong secret answer five times. Every attempt should get the level's normal reply.

It's broken if: a level's cheat-sheet solution no longer returns the result key; or a level answers An Error Occurred! You must be getting funky!, hangs for about 5 seconds, or mentions Connection is not available after the repeated inputs, and keeps doing so until Tomcat restarts.

Part of #857. Follows #998, and touches none of the same files.

What

SessionManagement6SecretQuestion's GET is the injectable lookup. It leaked its connection on a malformed email, so 3 bad lookups took the level offline until Tomcat restarted.
SessionManagement6, SessionManagement7 and SessionManagement7SecretQuestion closed their connection only when no exception was thrown.

Every Connection, PreparedStatement and ResultSet is now in try-with-resources.

The levels are exactly as vulnerable as before

Only resource handling changed. The conversion is the same as in the session management 2/3/5 PR, with one variable per statement (commitStmt, addressStmt / addressResult). The secret-question lookup still shows the player the raw SQL exception, which is part of the level. Review with git diff -w origin/dev.

Tests

SessionManagement6And7PoolLeakIT sends 5 malformed secret-question lookups, checking for "SQL syntax", then a valid one. It also makes 5 calls each to the SM6 and SM7 login and secret-answer servlets. On dev it fails 3 of 5, for example:

Malformed email 4 did not fail (expected "SQL syntax"): java.sql.SQLTransientConnectionException: ChallengePool-… - Connection is not available, request timed out after 5…

With this change, all 5 pass.

Every step of test.yml passed locally (lint, build, PGP check, compose build, Tomcat config check, unit tests, integration tests).

🤖 Generated with Claude Code

…art of #857) Each challenge schema has its own connection pool of three connections. These servlets closed their connection only when no exception was thrown. The Session Management 6 secret-question lookup is injectable, so three malformed email addresses took the level offline until Tomcat was restarted. Open every Connection, PreparedStatement and ResultSet in try-with-resources. Statements that were reassigned to one variable now each get their own resource (commitStmt, addressStmt). Only resource handling changes: the queries, the password filter, the messages (including the SQL error the secret-question lookup shows) and the catch block each exception reaches are the same as before. SessionManagement6And7PoolLeakIT calls each servlet five times, five malformed lookups included, and checks every well-formed request still reaches the database. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

🟢 Approval recommended

The scoped resource-lifecycle changes preserve challenge behavior, include regression coverage, and have no identified blocking issues.

No open issues 🎉

What changed in this PR

Closes JDBC resources in the Session Management 6 and 7 challenges as part of #857’s connection-leak cleanup, while preserving the intentionally vulnerable training behavior.

Changes:

Converts connections, statements, and result sets in four servlets to try-with-resources.
Preserves existing queries, validation, and player-facing SQL error messages.
Adds integration tests for repeated malformed lookups and repeated login and secret-answer requests.

File	Description
`src/main/java/servlets/module/challenge/SessionManagement7SecretQuestion.java`	Automatically closes secret-answer query resources.
`src/main/java/servlets/module/challenge/SessionManagement7.java`	Automatically closes login and address-lookup resources.
`src/main/java/servlets/module/challenge/SessionManagement6SecretQuestion.java`	Closes lookup and answer-query resources, including on SQL errors.
`src/main/java/servlets/module/challenge/SessionManagement6.java`	Automatically closes login and address-lookup resources.
`src/it/java/servlets/module/challenge/SessionManagement6And7PoolLeakIT.java`	Adds five repeated-request tests for challenge-pool availability.

_{Review effort: Balanced}

ismisepaul · 2026-10-25T04:35:52Z

+                      new String(
+                          "<h2 class='title'>"
+                              + bundle.getString("question.badAnswer")
+                              + "</h2><p>"
+                              + bundle.getString("question.whoAreYou"));


This line is unchanged on dev. This PR only re-indents it inside the new try-with-resources block, and in these training servlets the PR changes resource handling only. Tracked for a separate clean-up in #999.

ismisepaul · 2026-10-25T04:35:54Z

+                        new String(
+                            "<h2 class='title'>"
+                                + bundle.getString("question.badAnswer")
+                                + "</h2><p>"
+                                + bundle.getString("question.whoAreYou")
+                                + "</p>");


This line is unchanged on dev. This PR only re-indents it inside the new try-with-resources block, and in these training servlets the PR changes resource handling only. Tracked for a separate clean-up in #999.

ismisepaul · 2026-10-25T04:35:56Z

+                    // rs.getString(1) contains the question for the user to answer. This question
+                    // is asked in English as it must be answered in English to successfully pass
+                    // the level
+                    htmlOutput = new String(Encode.forHtml(rs.getString(1)));


This line is unchanged on dev. This PR only re-indents it inside the new try-with-resources block, and in these training servlets the PR changes resource handling only. Tracked for a separate clean-up in #999.

ismisepaul added this to the 4.0.0 milestone Oct 25, 2026

ismisepaul added Bug Levels labels Oct 25, 2026

Copilot AI balanced review requested due to automatic review settings October 25, 2026 00:59

github-project-automation Bot added this to Security Shepherd Oct 25, 2026

github-project-automation Bot moved this to Backlog in Security Shepherd Oct 25, 2026

ismisepaul moved this from Backlog to Next Release Backlog in Security Shepherd Oct 25, 2026

Copilot started reviewing on behalf of ismisepaul October 25, 2026 01:00

View session

Copilot AI reviewed Oct 25, 2026

View reviewed changes

github-code-quality Bot found potential problems Oct 25, 2026

View reviewed changes

ismisepaul mentioned this pull request Oct 25, 2026

fix: close JDBC resources in CSRF, crypto, URL access and misconfiguration challenges (part of #857) #1016

Open

ismisepaul added the Needs Manual Testing Must be tested by hand from the PR's branch (steps in the PR) before merging label Oct 25, 2026

Navigation Menu

Repository navigation

fix: close JDBC resources in Session Management 6 and 7 challenges (part of #857) - #1005

fix: close JDBC resources in Session Management 6 and 7 challenges (part of #857)#1005
ismisepaul wants to merge 1 commit into
devfrom
dev#857-session-management-6-7

ismisepaul commented Oct 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

ismisepaul Oct 25, 2026

Uh oh!

ismisepaul Oct 25, 2026

Uh oh!

ismisepaul Oct 25, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ismisepaul commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Manual testing required before merging

What

The levels are exactly as vulnerable as before

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

ismisepaul Oct 25, 2026

Choose a reason for hiding this comment

Uh oh!

ismisepaul Oct 25, 2026

Choose a reason for hiding this comment

Uh oh!

ismisepaul Oct 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ismisepaul commented Oct 25, 2026 •

edited

Loading