Runnable lab

01Get it

Two downloads. The source zip is what you should work from; the prebuilt WAR is for when you only want to see the behaviour.

# verify, then build, then run
$ shasum -a 256 -c SHA256SUMS.txt
$ unzip javasec-lab.zip && cd javasec-lab

$ mvn test        # 17 tests, no container required
$ mvn package     # -> target/javasec-lab.war
$ cp target/javasec-lab.war $CATALINA_HOME/webapps/
$ open http://127.0.0.1:8081/javasec-lab/

02The pairs, and what each proves

Every route exists as /vulnerable and /fixed. Send the same input to both and compare. These are the actual responses from a local run on Tomcat 10.1.61.

SQL injection

The vulnerable method concatenates, so the input decides the query's structure. The fix binds a parameter, so the same bytes are only ever data.

$ curl "…/lab/sqli/vulnerable?owner=' OR '1'='1"
rows: 4
  1  paul  Asha Menon  paul: negotiating salary
  2  paul  Ravi Kumar  paul: personal
  3  dana  Mei Lin     dana: do not contact before Q3
  4  dana  Tom Reed    dana: confidential

$ curl "…/lab/sqli/fixed?owner=' OR '1'='1"
rows: 0          # a literal owner name, matching nobody

Note what the fixed version does not do: it does not reject the input, sanitise it, or log an attack. It treats it as what it is — an owner name that happens to contain punctuation — and finds no such owner. That is what a correct fix looks like.

Broken object-level authorization

The instructive one, because the vulnerable query is already parameterised. There is no dangerous API anywhere in it. What is missing is a clause.

$ curl "…/lab/idor/vulnerable?id=3"      # id 3 belongs to dana
rows: 1
  3  dana  Mei Lin  dana: do not contact before Q3

$ curl "…/lab/idor/fixed?id=3"           # asked as paul
rows: 0

No grep finds this. The vulnerable and correct code differ by AND owner = ?, and both read as ordinary repository methods. It is the class that most resists tooling and most needs a human reading endpoints against their data model.

Cross-site scripting, three contexts

$ curl "…/lab/xss/vulnerable?q=<script>alert(1)</script>"
<p>No results for <script>alert(1)</script></p>

$ curl "…/lab/xss/fixed?q=<script>alert(1)</script>"
<p>No results for &lt;script&gt;alert(1)&lt;/script&gt;</p>

The attribute route needs no tags at all — closing the quote is enough, which is why “strip <script>” fails:

$ curl '…/lab/xss/attribute-vulnerable?q=" onmouseover=alert(1) x="'
<input value="" onmouseover=alert(1) x="">

Path traversal

$ curl -i "…/lab/traversal/fixed?name=../../../../etc/hosts"
HTTP/1.1 403

PathResolver carries three methods on purpose: the vulnerable concatenation, a naive sanitiser that strips ../ once, and the fix. The sanitiser is there to be defeated, because that is the version most likely to be in the code you are reviewing.

Deserialization

SessionCodec.Tripwire is a Serializable whose readObject sets a static flag. A real gadget chain reaches Runtime through library classes; the mechanism being shown — code running during the read, before the caller can check anything — is identical.

byte[] hostile = codec.write(new Tripwire());

codec.readVulnerable(hostile);
// Tripwire.fired == true   — it ran. The cast would throw afterwards.

codec.readFiltered(hostile);
// InvalidClassException, and Tripwire.fired == false — nothing ran.

codec.readWeaklyFiltered(hostile);     // filter: "!org.apache.commons.**"
// Tripwire.fired == true   — a deny-list permits everything it did not name.

03Tests that assert both halves

Every test proves the vulnerable form is genuinely exploitable and that the fix resists the same input.

$ mvn test
Tests run: 2, Failures: 0  — VulnerabilityTest$Sqli
Tests run: 2, Failures: 0  — VulnerabilityTest$Idor
Tests run: 4, Failures: 0  — VulnerabilityTest$Traversal
Tests run: 5, Failures: 0  — VulnerabilityTest$Xss
Tests run: 4, Failures: 0  — VulnerabilityTest$Deser
Tests run: 17, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

04Three things building it taught

These are not hypotheticals. Each one happened while assembling this lab, and each is in the source as a comment.

A green test suite does not predict deployment

mvn test passed. The WAR then returned 500 on every database route:

java.sql.SQLException: No suitable driver found for jdbc:h2:mem:javaseclab
  at dev.luap.javasec.Db.connect(Db.java:24)
  at dev.luap.javasec.ContactRepository.findByOwnerFixed(…)
  at dev.luap.javasec.LabServlet.doGet(…)
  at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(…)
  at org.apache.catalina.core.StandardWrapperValve.invoke(…)
  at org.apache.catalina.connector.CoyoteAdapter.service(…)
  at org.apache.coyote.http11.Http11Processor.service(…)
  at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(…)

Read that trace bottom-up and it is exactly the pipeline on the Tomcat page: endpoint, processor, adapter, valves, filter chain, servlet.

The cause is class loading. DriverManager lives in java.sql, loaded by the platform class loader. Under Maven everything is on one flat classpath and the JDBC ServiceLoader finds org.h2.Driver. Inside a WAR the driver is in WEB-INF/lib, visible only to the webapp loader, which the platform loader cannot see. The fix registers it from the right side of the boundary:

static {
    Class.forName("org.h2.Driver");   // runs under the WEBAPP loader
}

In production the better answer is a container-managed DataSource: driver in CATALINA_HOME/lib, looked up over JNDI, pool shared across applications.

A terse error response must still be logged

The servlet originally did this, which is half right:

catch (SQLException e) {
    resp.sendError(500, "query failed");   // and nothing else
}

Terse to the client is correct — an exception message echoed to the browser is an information-disclosure sink. But logging nothing made the driver failure above invisible: the response said query failed and catalina.out was silent. Both halves are required, and omitting the second is the more common mistake.

catch (SQLException e) {
    log("query failed on route " + route, e);   // detailed, server-side
    resp.sendError(500, "query failed");          // terse, client-side
}

One more wrinkle: GenericServlet.log() writes to the context log, so it lands in logs/localhost.<date>.log and not catalina.out. Knowing which file to read is half of diagnosing a servlet.

Registering a driver leaks the class loader

DriverManager outlives your application and now holds a reference to a class your loader defined. That single reference pins the whole WebappClassLoader, and with it every class it ever loaded, so nothing is collected on undeploy. LabLifecycle cleans up:

for (Driver d : Collections.list(DriverManager.getDrivers())) {
    // only drivers THIS application loaded — touching the container's
    // would break every other deployed application
    if (d.getClass().getClassLoader() == mine) {
        DriverManager.deregisterDriver(d);
    }
}

Without it: OutOfMemoryError: Metaspace after a handful of redeploys. It reads as a sizing problem and is a class-loading bug.

05Two assertions that were wrong

Both were caught by running the suite rather than reading it. They are kept in the tests because the corrected version teaches more than a test that was right first time.

....// is not a traversal

The first version expected resolveFixed("....//....//etc/hosts") to throw. It does not, and it should not. .... is a legal four-character directory name; normalize() finds no .. to collapse, so the path stays inside the base directory and the resolver correctly permits it.

It is dangerous only against a sanitiser that rewrites ../ and thereby creates the sequence it was removing:

resolveFixed            ("....//....//etc/hosts")  -> inside base, allowed
resolveNaivelySanitised ("....//....//etc/hosts")  -> ../../etc/hosts, escaped

The lesson is about where the bug lives. The payload does not attack the filesystem; it attacks the cleanup function.

Encode.forHtml does encode the double quote

The first version asserted that HTML-encoding a value inside a <script> string literal left a quote break-out intact. It does not — forHtml turns " into ", so that payload is stopped by accident.

input  ";alert(1);//    forHtml -> ";alert(1);//     forJavaScript -> \x22;alert(1);\/\/
input  \                forHtml -> \                    forJavaScript -> \\

The genuine gap is the backslash, which forHtml leaves untouched. A trailing backslash escapes the closing quote, so the string never terminates and the following code is swallowed into it.

06Exercises

The lab is deliberately incomplete. Each of these is a working change you can make and verify with a test.

1. Add the SSRF pair. A route that fetches a URL, and a fixed version that allow-lists scheme and host, resolves DNS, rejects private ranges and pins the resolved IP. Assert that http://169.254.169.254/ and a redirect to it are both refused.
2. Add mass assignment. Bind a form to the Contact record directly, show owner being overwritten, then fix it with a DTO that declares only the editable fields.
3. Add the filter-ordering bypass. Map an auth filter to /lab/admin/*, add a public route that forwards into it, and prove the filter is skipped. Then add <dispatcher>FORWARD</dispatcher> and prove it is not.
4. Make LabServlet thread-unsafe on purpose. Move repo to a mutable instance field holding the last result, drive it with concurrent requests for different owners, and watch one user's rows appear in another's response.
5. Break the ObjectInputFilter. Remove the trailing !* and show that Tripwire is accepted again.

For each: write the failing test first, make it pass, then write the test that proves the fix. If you cannot make the vulnerable version fail a test, you have not reproduced the bug — and a fix for a bug you did not reproduce is a guess.