Skip to content

Conversation

@zeropath-ai
Copy link

@zeropath-ai zeropath-ai bot commented Feb 3, 2025

Summary

  • The Vulnerability Description: The GetTicket function failed to validate whether the requester was authorized to access the ticket data. By manipulating the id parameter in the request, an attacker could retrieve tickets belonging to other users, resulting in an insecure direct object reference (IDOR) vulnerability.

  • This Fix: The patch ensures that the GetTicket function now validates the identity of the requester by comparing the authenticated user’s ID (retrieved from the session or token) with the id parameter in the request. Unauthorized requests are denied with appropriate HTTP status codes (401 Unauthorized or 403 Forbidden).

  • The Cause of the Issue: The original implementation retrieved and processed the id parameter directly from the request without verifying that the authenticated user was authorized to access the data associated with it. This lack of access control checks allowed unauthorized users to retrieve other users' tickets.

  • The Patch Implementation: The function now retrieves the authenticated user ID from the context (e.g., session or token) and validates it against the request’s id parameter. If the IDs do not match, the function returns a 403 Forbidden response, preventing unauthorized access. Additionally, a 401 Unauthorized response is returned if no authenticated user ID is found.

Vulnerability Details

  • Vulnerability Class: Insecure Direct Object Reference (IDOR)
  • Severity: 9.2
  • Affected File: owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/handlers.go
  • Vulnerable Lines: 17-37

Code Snippets

diff --git a/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/handlers.go b/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/handlers.go
index ea71f28d..b648b1fa 100644
--- a/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/handlers.go
+++ b/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/handlers.go
@@ -13,13 +13,25 @@ func HealthCheck(c echo.Context) error {
 	return c.String(http.StatusOK, "WORKING\n")
 }
 
-// GetTicket returns the userID ticket.
+// GetTicket returns the authenticated user's ticket.
 func GetTicket(c echo.Context) error {
-	id := c.Param("id")
-	userDataQuery := map[string]interface{}{"userID": id}
+	// Get authenticated user's ID from the session/token
+	authenticatedUserID := c.Get("user")
+	if authenticatedUserID == nil {
+		return c.JSON(http.StatusUnauthorized, map[string]string{"result": "error", "details": "Authentication required"})
+	}
+
+	// Get requested user ID from path parameter
+	requestedID := c.Param("id")
+	
+	// Verify user is accessing their own ticket
+	if authenticatedUserID.(string) != requestedID {
+		return c.JSON(http.StatusForbidden, map[string]string{"result": "error", "details": "Access denied"})
+	}
+
+	userDataQuery := map[string]interface{}{"userID": requestedID}
 	userDataResult, err := db.GetUserData(userDataQuery)
 	if err != nil {
-		// could not find this user in MongoDB (or MongoDB err connection)
 		return c.JSON(http.StatusBadRequest, map[string]string{"result": "error", "details": "Error finding this UserID."})
 	}
 

How to Modify the Patch

You can modify this patch by using one of the two methods outlined below. We recommend using the @zeropath-ai bot for updating the code. If you encounter any bugs or issues with the patch, please report them here.

Ask @zeropath-ai!

To request modifications, please post a comment beginning with @zeropath-ai and specify the changes required.

@zeropath-ai will then implement the requested adjustments and commit them to the specified branch in this pull request. Our bot is capable of managing changes across multiple files and various development-related requests.

Manually Modify the Files

# Checkout created branch:
git checkout zvuln_fix_insecure_direct_object_reference_idor_1738551570268483

# if vscode is installed run (or use your favorite editor / IDE):
code owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/handlers.go

# Add, commit, and push changes:
git add -A
git commit -m "Update generated patch with x, y, and z changes."
git push zvuln_fix_insecure_direct_object_reference_idor_1738551570268483

@r0path
Copy link
Member

r0path commented Feb 3, 2025

@zeropath-ai Please create a reusable authentication helper function in auth.go that validates the calling user's auth. Then import and use this authentication function at the start of the current function to verify the user has proper authorization before proceeding with the rest of the logic.

@zeropath-ai
Copy link
Author

zeropath-ai bot commented Feb 3, 2025

As per your comment, the following code change has been committed:

diff --git a/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/auth.go b/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/auth.go
new file mode 100644
index 00000000..6a4ffef1
--- /dev/null
+++ b/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/auth.go
@@ -0,0 +1,26 @@
+package handlers
+
+import (
+	"github.com/labstack/echo"
+	"net/http"
+)
+
+// ValidateUserAccess checks if the authenticated user has access to the requested resource
+func ValidateUserAccess(c echo.Context, requestedID string) error {
+	authenticatedUserID := c.Get("user")
+	if authenticatedUserID == nil {
+		return echo.NewHTTPError(http.StatusUnauthorized, map[string]string{
+			"result": "error", 
+			"details": "Authentication required",
+		})
+	}
+
+	if authenticatedUserID.(string) != requestedID {
+		return echo.NewHTTPError(http.StatusForbidden, map[string]string{
+			"result": "error",
+			"details": "Access denied",
+		})
+	}
+
+	return nil
+}
diff --git a/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/handlers.go b/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/handlers.go
index b648b1fa..ead3a8bd 100644
--- a/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/handlers.go
+++ b/owasp-top10-2021-apps/a1/ecommerce-api/app/handlers/handlers.go
@@ -15,18 +15,10 @@ func HealthCheck(c echo.Context) error {
 
 // GetTicket returns the authenticated user's ticket.
 func GetTicket(c echo.Context) error {
-	// Get authenticated user's ID from the session/token
-	authenticatedUserID := c.Get("user")
-	if authenticatedUserID == nil {
-		return c.JSON(http.StatusUnauthorized, map[string]string{"result": "error", "details": "Authentication required"})
-	}
-
-	// Get requested user ID from path parameter
 	requestedID := c.Param("id")
 	
-	// Verify user is accessing their own ticket
-	if authenticatedUserID.(string) != requestedID {
-		return c.JSON(http.StatusForbidden, map[string]string{"result": "error", "details": "Access denied"})
+	if err := ValidateUserAccess(c, requestedID); err != nil {
+		return err
 	}
 
 	userDataQuery := map[string]interface{}{"userID": requestedID}

Click here to report a bug or a broken patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant