From a058b1596f54540f26dc21878c0fc4a4662e57ec Mon Sep 17 00:00:00 2001 From: beanbeah <24713371+beanbeah@users.noreply.github.com> Date: Sun, 9 Aug 2026 11:35:02 -0700 Subject: [PATCH 1/2] fix: prevent integer overflow in PoorValidation2 cost calculation PoorValidation2's shopping-cart cost math used plain int arithmetic (pineappleAmount*30, orangeAmount*3000, etc.) and validateAmount() only clamped the lower bound (negative amounts -> 0). A sufficiently large positive amount (e.g. orangeAmount=1000000 -> orangeCost=3,000,000,000) overflows a 32-bit int and wraps negative, driving finalCost <= 0 while orangeAmount > 0 and unlocking the free-oranges success response without a legitimate order. Fix: validateAmount() now also clamps the upper bound to a new MAX_ITEM_AMOUNT=1000 constant, and all cost arithmetic (pineappleCost/orangeCost/appleCost/bananaCost/finalCost) is widened from int to long as defense-in-depth so the multiplication itself cannot wrap even if the clamp were ever loosened. Added PoorValidation2IT covering: the original overflow exploit no longer produces the free-oranges response, a larger multi-field overflow attempt also fails, and a legitimate order of 1 of each item still completes with the correct $3090 total. Co-Authored-By: Claude Sonnet 5 --- .../module/challenge/PoorValidation2IT.java | 119 ++++++++++++++++++ .../module/challenge/PoorValidation2.java | 24 +++- 2 files changed, 137 insertions(+), 6 deletions(-) create mode 100644 src/it/java/servlets/module/challenge/PoorValidation2IT.java diff --git a/src/it/java/servlets/module/challenge/PoorValidation2IT.java b/src/it/java/servlets/module/challenge/PoorValidation2IT.java new file mode 100644 index 000000000..56cf313eb --- /dev/null +++ b/src/it/java/servlets/module/challenge/PoorValidation2IT.java @@ -0,0 +1,119 @@ +package servlets.module.challenge; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import dbProcs.GetterIT; +import dbProcs.Setter; +import java.io.IOException; +import java.sql.SQLException; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.mock.web.MockServletConfig; +import testUtils.TestProperties; + +public class PoorValidation2IT { + + private static String applicationRoot = new String(); + private static String USERNAME = "lessonTester"; + private static String LANG = "en_GB"; + + private static final Logger log = LogManager.getLogger(PoorValidation2IT.class); + + private MockHttpServletRequest request; + private MockHttpServletResponse response; + + /** Creates DB or Restores DB to Factory Defaults before running tests */ + @BeforeAll + public static void resetDatabase() throws IOException, SQLException { + TestProperties.setTestPropertiesFileDirectory(log); + + TestProperties.createMysqlResource(); + + TestProperties.ensureSchemaReady(log); + TestProperties.reseedTestData(); + } + + @BeforeEach + public void setup() { + request = new MockHttpServletRequest(); + response = new MockHttpServletResponse(); + + // Open All modules + if (!Setter.openAllModules(applicationRoot, false)) { + fail("Could not Mark All Modules As Open"); + } + } + + private String submitOrder( + String pineappleAmount, String orangeAmount, String appleAmount, String bananaAmount) + throws Exception { + String servletClassName = "PoorValidation2"; + log.debug("Creating " + servletClassName + " Servlet Instance"); + PoorValidation2 servlet = new PoorValidation2(); + servlet.init(new MockServletConfig(servletClassName)); + + request.addParameter("pineappleAmount", pineappleAmount); + request.addParameter("orangeAmount", orangeAmount); + request.addParameter("appleAmount", appleAmount); + request.addParameter("bananaAmount", bananaAmount); + + log.debug("Running doPost"); + servlet.doPost(request, response); + + return response.getContentAsString(); + } + + private void signIn() throws Exception { + GetterIT.verifyTestUser(applicationRoot, USERNAME, USERNAME); + log.debug("Signing in as " + USERNAME + " Through LoginServlet"); + TestProperties.loginDoPost(log, request, response, USERNAME, USERNAME, null, LANG); + if (response.getCookie("token") == null) { + fail("No CSRF Token Was Returned from Login Servlet"); + } + request.setCookies(response.getCookies()); + } + + /** + * Exploit attempt: previously, a huge positive orangeAmount overflowed the int-based cost + * arithmetic (orangeAmount * 3000) around to a negative number, driving finalCost below zero + * and unlocking the free-oranges response without a legitimate zero/low-cost order. With the + * fix (amount clamped to a max + long arithmetic) this must no longer succeed. + */ + @Test + public void testIntegerOverflowExploitFails() throws Exception { + signIn(); + String servletResponse = submitOrder("0", "1000000", "0", "0"); + assertFalse( + servletResponse.contains("Oranges were free"), + "Integer-overflow exploit unexpectedly produced the free-oranges response: " + + servletResponse); + } + + /** A second, even larger overflow attempt across multiple fields must also fail. */ + @Test + public void testLargeMultiFieldOverflowExploitFails() throws Exception { + signIn(); + String servletResponse = submitOrder("2000000000", "2000000000", "2000000000", "2000000000"); + assertFalse( + servletResponse.contains("Oranges were free"), + "Multi-field overflow exploit unexpectedly produced the free-oranges response: " + + servletResponse); + } + + /** Legitimate small order (one of each item) must still complete normally with the right total. */ + @Test + public void testLegitimateOrderStillWorks() throws Exception { + signIn(); + String servletResponse = submitOrder("1", "1", "1", "1"); + assertTrue( + servletResponse.contains("3090"), + "Legitimate order did not compute the expected total: " + servletResponse); + } +} diff --git a/src/main/java/servlets/module/challenge/PoorValidation2.java b/src/main/java/servlets/module/challenge/PoorValidation2.java index 818084430..131e28705 100644 --- a/src/main/java/servlets/module/challenge/PoorValidation2.java +++ b/src/main/java/servlets/module/challenge/PoorValidation2.java @@ -79,16 +79,18 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) int bananaAmount = validateAmount(Integer.parseInt(request.getParameter("bananaAmount"))); log.debug("bananaAmount - " + bananaAmount); - // Working out costs - int pineappleCost = pineappleAmount * 30; - int orangeCost = orangeAmount * 3000; - int appleCost = appleAmount * 45; - int bananaCost = bananaAmount * 15; + // Working out costs. Amounts are widened to long before multiplying so that even + // if validateAmount()'s clamp were ever loosened, the arithmetic itself cannot wrap + // an int around to a negative total. + long pineappleCost = (long) pineappleAmount * 30; + long orangeCost = (long) orangeAmount * 3000; + long appleCost = (long) appleAmount * 45; + long bananaCost = (long) bananaAmount * 15; htmlOutput = new String(); // Work Out Final Cost - int finalCost = pineappleCost + orangeCost + bananaCost + appleCost; + long finalCost = pineappleCost + orangeCost + bananaCost + appleCost; // Output Order htmlOutput = @@ -126,9 +128,19 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) } } + /** + * Amounts submitted by the client are clamped to a sane, bounded range so that neither a + * negative quantity nor an absurdly large one (previously able to overflow the int-based cost + * arithmetic into a negative total and trigger the free-oranges response) can reach the cost + * calculation below. + */ + private static final int MAX_ITEM_AMOUNT = 1000; + private static int validateAmount(int amount) { if (amount < 0) { amount = 0; + } else if (amount > MAX_ITEM_AMOUNT) { + amount = MAX_ITEM_AMOUNT; } return amount; } From e7b57931c10bde701c74eaf764f868b5bea6afd3 Mon Sep 17 00:00:00 2001 From: beanbeah <24713371+beanbeah@users.noreply.github.com> Date: Sun, 9 Aug 2026 11:42:50 -0700 Subject: [PATCH 2/2] style: apply spotless formatting (google-java-format comment reflow) Co-Authored-By: Claude Sonnet 5 --- .../servlets/module/challenge/PoorValidation2IT.java | 10 ++++++---- .../servlets/module/challenge/PoorValidation2.java | 8 ++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/it/java/servlets/module/challenge/PoorValidation2IT.java b/src/it/java/servlets/module/challenge/PoorValidation2IT.java index 56cf313eb..6ae4383d8 100644 --- a/src/it/java/servlets/module/challenge/PoorValidation2IT.java +++ b/src/it/java/servlets/module/challenge/PoorValidation2IT.java @@ -82,9 +82,9 @@ private void signIn() throws Exception { /** * Exploit attempt: previously, a huge positive orangeAmount overflowed the int-based cost - * arithmetic (orangeAmount * 3000) around to a negative number, driving finalCost below zero - * and unlocking the free-oranges response without a legitimate zero/low-cost order. With the - * fix (amount clamped to a max + long arithmetic) this must no longer succeed. + * arithmetic (orangeAmount * 3000) around to a negative number, driving finalCost below zero and + * unlocking the free-oranges response without a legitimate zero/low-cost order. With the fix + * (amount clamped to a max + long arithmetic) this must no longer succeed. */ @Test public void testIntegerOverflowExploitFails() throws Exception { @@ -107,7 +107,9 @@ public void testLargeMultiFieldOverflowExploitFails() throws Exception { + servletResponse); } - /** Legitimate small order (one of each item) must still complete normally with the right total. */ + /** + * Legitimate small order (one of each item) must still complete normally with the right total. + */ @Test public void testLegitimateOrderStillWorks() throws Exception { signIn(); diff --git a/src/main/java/servlets/module/challenge/PoorValidation2.java b/src/main/java/servlets/module/challenge/PoorValidation2.java index 131e28705..380ac9c0b 100644 --- a/src/main/java/servlets/module/challenge/PoorValidation2.java +++ b/src/main/java/servlets/module/challenge/PoorValidation2.java @@ -129,10 +129,10 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) } /** - * Amounts submitted by the client are clamped to a sane, bounded range so that neither a - * negative quantity nor an absurdly large one (previously able to overflow the int-based cost - * arithmetic into a negative total and trigger the free-oranges response) can reach the cost - * calculation below. + * Amounts submitted by the client are clamped to a sane, bounded range so that neither a negative + * quantity nor an absurdly large one (previously able to overflow the int-based cost arithmetic + * into a negative total and trigger the free-oranges response) can reach the cost calculation + * below. */ private static final int MAX_ITEM_AMOUNT = 1000;