Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions web/src/main/java/com/example/server/server2.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package com.example.server;
import javax.servlet.ServletException;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.sql.*;

public class Server extends HttpServlet {

static final String DB_URL = "jdbc:mysql://localhost/users";
static Connection conn;

@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
Cookie c = new Cookie("uid", req.getSession().getId());
// For older browsers?
c.setSecure(false);
resp.addCookie(c);
Comment on lines +17 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`setSecure(false)` on `uid` cookie exposes session identifier


c carries the session ID and explicitly disables the Secure flag. Any HTTP downgrade or misrouted plaintext request can leak credentials and allow takeover.
Set c.setSecure(true) and c.setHttpOnly(true), and add SameSite via Set-Cookie attributes to reduce cross-site abuse

resp.setHeader("Access-Control-Allow-Origin", "*");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wildcard `Access-Control-Allow-Origin` enables data theft


The use of resp.setHeader("Access-Control-Allow-Origin", "*") permits any external website to make cross-origin requests to the server. This breaks the Same Origin Policy, allowing attackers to retrieve sensitive data through the victim's browser.
Use a domain whitelist to restrict Access-Control-Allow-Origin to trusted domains. Dynamically set the header based on the Origin request header instead of using the wildcard *.


Boolean b = Boolean.parseBoolean(req.getParameter("winCondition"));
int ticketNumber = Integer.parseInt(req.getParameter("ticket"));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`Integer.parseInt` on `ticket` can throw runtime exception


ticketNumber parsing assumes valid numeric input. A malformed or absent parameter crashes the request path before SQL handling.
Wrap parsing in validation, reject invalid values with 400, and return early to keep error semantics predictable

try {
Statement s = conn.createStatement();
s.execute("SELECT userName, isWin FROM users WHERE uid = " + ticketNumber + ";");
Comment on lines +24 to +27

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`ticket` parameter lookup enables IDOR user data access


doGet trusts client-supplied ticketNumber to fetch user data, but performs no authorization check against the caller’s session. An attacker can request arbitrary uid values and access another user’s result.
Add a server-side ownership check before querying, binding the session identity to allowed uid values and rejecting mismatches with 403

ResultSet r = s.getResultSet();
Comment on lines +26 to +28

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`Statement` and `ResultSet` not closed leak database resources


Each call allocates s and r but never closes them. Under load, open cursors accumulate and database operations eventually fail.
Use try-with-resources for Statement and ResultSet so cleanup occurs on normal and exceptional paths


if (r.getBoolean("isWin") && b) {
resp.getWriter().write("You win, " + r.getString("userName"));
} else {
resp.getWriter().write("You lose, " + r.getString("userName"));
Comment on lines +28 to +33

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`ResultSet` reads before `next()` can throw exceptions


r is consumed immediately after getResultSet() without checking row existence. Empty results trigger SQLException, and valid rows may be skipped incorrectly depending on driver behavior.
Call if (r.next()) before reads, and return 404 or a safe message when no record exists

}
} catch (SQLException throwables) {
throwables.printStackTrace();
}
resp.setStatus(200);
Comment on lines +35 to +38

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`setStatus(200)` after exceptions hides backend failures


doGet catches SQLException and still returns 200. Clients receive false positives, retries may stop, and monitoring misses real failures.
Set an error status like 500 in the catch block and return immediately after writing a safe error message

}

@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
super.doPost(req, resp);
}

@Override
public void destroy() {
super.destroy();
Comment on lines +47 to +48

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`destroy()` omits `conn.close()` causing connection leak


destroy does not release conn. Redeploy cycles can accumulate abandoned connections and break subsequent initializations.
Add a guarded close in destroy, handle SQLException, then null out conn to avoid reuse

}

@Override
public void init() throws ServletException {
super.init();

try {
conn = DriverManager.getConnection(DB_URL, "user", "");
} catch (SQLException throwables) {
throwables.printStackTrace();
}

}
}