Race condition in counting number of attempts #567 (#697)

Add version to Hibernate mapping so we get optimistic locking this solves
number of parallel calls trying to update/guess and mess with the lesson
counter

webgoat-container/src/main/java/org/owasp/webgoat/users/LessonTracker.java

@@ -54,6 +54,8 @@ public class LessonTracker {
     private final Set<Assignment> allAssignments = new HashSet<>();
     @Getter
     private int numberOfAttempts = 0;
+    @Version
+    private Integer version;
 
     private LessonTracker() {
         //JPA

webgoat-container/src/main/resources/db/container/V2__version.sql

@@ -0,0 +1 @@
+ALTER TABLE CONTAINER.LESSON_TRACKER ADD VERSION INTEGER;

webgoat-integration-tests/src/test/java/org/owasp/webgoat/IntegrationTest.java

@@ -37,9 +37,9 @@ public abstract class IntegrationTest {
 
     @BeforeClass
     public static void beforeAll() {
-if (WG_SSL) {
+        if (WG_SSL) {
-    		WEBGOAT_URL = WEBGOAT_URL.replace("http:","https:");
+            WEBGOAT_URL = WEBGOAT_URL.replace("http:", "https:");
-    	}
+        }
         if (!started) {
             started = true;
             if (!isAlreadyRunning(WG_PORT)) {
@@ -84,7 +84,7 @@ if (WG_SSL) {
                 .relaxedHTTPSValidation()
                 .formParam("username", webgoatUser)
                 .formParam("password", "password")
-                    .post(url("login")).then()
+                .post(url("login")).then()
                 .cookie("JSESSIONID")
                 .statusCode(302)
                 .extract().header("Location");
@@ -236,7 +236,8 @@ if (WG_SSL) {
                         .statusCode(200)
                         .extract().path("lessonCompleted"), CoreMatchers.is(expectedResult));
     }
-public void checkAssignmentWithGet(String url, Map<String, ?> params, boolean expectedResult) {
+
+    public void checkAssignmentWithGet(String url, Map<String, ?> params, boolean expectedResult) {
         Assert.assertThat(
                 RestAssured.given()
                         .when()
@@ -251,28 +252,28 @@ public void checkAssignmentWithGet(String url, Map<String, ?> params, boolean ex
 
     public String getWebGoatServerPath() throws IOException {
 
-    	//read path from server
+        //read path from server
         String result = RestAssured.given()
-        .when()
+                .when()
-        .relaxedHTTPSValidation()
+                .relaxedHTTPSValidation()
-        .cookie("JSESSIONID", getWebGoatCookie())
+                .cookie("JSESSIONID", getWebGoatCookie())
-        .get(url("/WebGoat/xxe/tmpdir"))
+                .get(url("/WebGoat/xxe/tmpdir"))
-        .then()
+                .then()
-        .extract().response().getBody().asString();
+                .extract().response().getBody().asString();
         result = result.replace("%20", " ");
         return result;
     }
 
     public String getWebWolfServerPath() throws IOException {
 
-    	//read path from server
+        //read path from server
         String result = RestAssured.given()
-        .when()
+                .when()
-        .relaxedHTTPSValidation()
+                .relaxedHTTPSValidation()
-        .cookie("WEBWOLFSESSION", getWebWolfCookie())
+                .cookie("WEBWOLFSESSION", getWebWolfCookie())
-        .get(webWolfUrl("/tmpdir"))
+                .get(webWolfUrl("/tmpdir"))
-        .then()
+                .then()
-        .extract().response().getBody().asString();
+                .extract().response().getBody().asString();
         result = result.replace("%20", " ");
         return result;
     }

webgoat-integration-tests/src/test/java/org/owasp/webgoat/ProgressRaceConditionTest.java

@@ -0,0 +1,43 @@
+package org.owasp.webgoat;
+
+import io.restassured.RestAssured;
+import io.restassured.response.Response;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+public class ProgressRaceConditionTest extends IntegrationTest {
+
+    @Test
+    public void runTests() throws InterruptedException {
+        startLesson("Challenge1");
+
+        Callable<Response> call = () ->
+                RestAssured.given()
+                        .when()
+                        .relaxedHTTPSValidation()
+                        .cookie("JSESSIONID", getWebGoatCookie())
+                        .formParams(Map.of("flag", "test"))
+                        .post(url("/challenge/flag/"));
+        ExecutorService executorService = Executors.newFixedThreadPool(20);
+        List<? extends Callable<Response>> flagCalls = IntStream.range(0, 20).mapToObj(i -> call).collect(Collectors.toList());
+        var responses = executorService.invokeAll(flagCalls);
+
+        //A certain amount of parallel calls should fail as optimistic locking in DB is applied
+        Assertions.assertThat(responses.stream().filter(r -> {
+            try {
+                return r.get().getStatusCode() == 500;
+            } catch (InterruptedException | ExecutionException e) {
+                throw new IllegalStateException(e);
+            }
+        }).count()).isGreaterThan(10);
+    }
+}