Fix: IDOR allows editing another user's profile via PUT /IDOR/profile/{userId}

pom.xml

@@ -238,7 +238,7 @@
     <dependency>
       <groupId>org.projectlombok</groupId>
       <artifactId>lombok</artifactId>
-      <version>1.18.36</version>
+      <version>1.18.46</version>
       <scope>provided</scope>
       <optional>true</optional>
     </dependency>

src/main/java/org/owasp/webgoat/lessons/idor/IDOREditOtherProfile.java

@@ -43,60 +43,34 @@ public AttackResult completed(
       @PathVariable("userId") String userId, @RequestBody UserProfile userSubmittedProfile) {
 
     String authUserId = (String) userSessionData.getValue("idor-authenticated-user-id");
-    // this is where it starts ... accepting the user submitted ID and assuming it will be the same
-    // as the logged in userId and not checking for proper authorization
-    // Certain roles can sometimes edit others' profiles, but we shouldn't just assume that and let
-    // everyone, right?
-    // Except that this is a vulnerable app ... so we will
-    UserProfile currentUserProfile = new UserProfile(userId);
-    if (userSubmittedProfile.getUserId() != null
-        && !userSubmittedProfile.getUserId().equals(authUserId)) {
-      // let's get this started ...
-      currentUserProfile.setColor(userSubmittedProfile.getColor());
-      currentUserProfile.setRole(userSubmittedProfile.getRole());
-      // we will persist in the session object for now in case we want to refer back or use it later
-      userSessionData.setValue("idor-updated-other-profile", currentUserProfile);
-      if (currentUserProfile.getRole() <= 1
-          && currentUserProfile.getColor().equalsIgnoreCase("red")) {
-        return success(this)
-            .feedback("idor.edit.profile.success1")
-            .output(currentUserProfile.profileToMap().toString())
-            .build();
-      }
-
-      if (currentUserProfile.getRole() > 1
-          && currentUserProfile.getColor().equalsIgnoreCase("red")) {
-        return failed(this)
-            .feedback("idor.edit.profile.failure1")
-            .output(currentUserProfile.profileToMap().toString())
-            .build();
-      }
 
-      if (currentUserProfile.getRole() <= 1
-          && !currentUserProfile.getColor().equalsIgnoreCase("red")) {
-        return failed(this)
-            .feedback("idor.edit.profile.failure2")
-            .output(currentUserProfile.profileToMap().toString())
-            .build();
-      }
+    // Proper authorization enforcement: the record being modified (identified by the path
+    // variable) and, if present, the userId carried inside the request body must both refer
+    // to the currently authenticated user. Neither the URL nor the JSON payload is trusted
+    // input for deciding whose record gets written - only the server-side session identity is.
+    boolean requestTargetsAuthenticatedUser =
+        authUserId != null
+            && authUserId.equals(userId)
+            && (userSubmittedProfile.getUserId() == null
+                || authUserId.equals(userSubmittedProfile.getUserId()));
 
-      // else
-      return failed(this)
-          .feedback("idor.edit.profile.failure3")
-          .output(currentUserProfile.profileToMap().toString())
-          .build();
-    } else if (userSubmittedProfile.getUserId() != null
-        && userSubmittedProfile.getUserId().equals(authUserId)) {
-      return failed(this).feedback("idor.edit.profile.failure4").build();
+    if (!requestTargetsAuthenticatedUser) {
+      // Whatever role/color values were supplied are irrelevant - this account has no
+      // authority to modify a profile that isn't its own.
+      return failed(this).feedback("idor.edit.profile.failure1").build();
     }
 
-    if (currentUserProfile.getColor().equals("black") && currentUserProfile.getRole() <= 1) {
-      return success(this)
-          .feedback("idor.edit.profile.success2")
-          .output(userSessionData.getValue("idor-updated-own-profile").toString())
-          .build();
-    } else {
-      return failed(this).feedback("idor.edit.profile.failure3").build();
+    // Legitimate path: the authenticated user is updating their own profile.
+    UserProfile currentUserProfile = new UserProfile(authUserId);
+    currentUserProfile.setUserId(authUserId);
+    if (userSubmittedProfile.getColor() != null) {
+      currentUserProfile.setColor(userSubmittedProfile.getColor());
     }
+    userSessionData.setValue("idor-updated-own-profile", currentUserProfile);
+
+    return success(this)
+        .feedback("idor.edit.profile.success2")
+        .output(currentUserProfile.profileToMap().toString())
+        .build();
   }
 }