Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Title: Fix authorization flaw in download_file_by_id function to ensure proper user access validation for resource downloads. #1

Merged

Conversation

zeropath-ai-dev[bot]
Copy link

Summary

  • The Vulnerability Description: The download_file_by_id function lacks proper authorization checks, allowing authenticated users to access files they do not have permission to view, by merely submitting a valid resource_id.

  • This Fix: Introduces a check to verify that the authenticated user belongs to the same organization as the owner of the resource, thereby preventing unauthorized access.

  • The Cause of the Issue: The function only checked for JWT validity without confirming if the user had the necessary permissions or relationship to access the requested resource.

  • The Patch Implementation: Added code to fetch the current user's organization ID from the JWT, and verified if it matches the organization ID of the agent owning the requested resource, returning a 403 error if it does not match.

Vulnerability Details

  • Vulnerability Class: Insecure Direct Object Reference (IDOR)
  • Severity: 8.2
  • Affected File: superagi/controllers/resources.py
  • Vulnerable Lines: 119-161

Code Snippets

diff --git a/superagi/controllers/resources.py b/superagi/controllers/resources.py
index ab83275e..a64767df 100644
--- a/superagi/controllers/resources.py
+++ b/superagi/controllers/resources.py
@@ -130,17 +130,30 @@ def download_file_by_id(resource_id: int,
 
     Raises:
         HTTPException (status_code=400): If the resource with the specified ID is not found.
+        HTTPException (status_code=403): If the user doesn't have permission to access this resource.
         HTTPException (status_code=404): If the file is not found.
 
     """
+    # Get current user's organization_id from JWT token
+    current_user_org_id = Authorize.get_jwt_subject()
 
+    # First check if resource exists
     resource = db.session.query(Resource).filter(Resource.id == resource_id).first()
-    download_file_path = resource.path
-    file_name = resource.name
-
     if not resource:
         raise HTTPException(status_code=400, detail="Resource Not found!")
 
+    # Get the agent that owns this resource
+    agent = db.session.query(Agent).filter(Agent.id == resource.agent_id).first()
+    if not agent:
+        raise HTTPException(status_code=400, detail="Associated agent not found!")
+
+    # Verify the authenticated user belongs to the same organization as the agent
+    if str(agent.organisation_id) != str(current_user_org_id):
+        raise HTTPException(status_code=403, detail="You don't have permission to access this resource")
+
+    download_file_path = resource.path
+    file_name = resource.name
+
     if resource.storage_type == StorageType.S3.value:
         bucket_name = get_config("BUCKET_NAME")
         file_key = resource.path

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-dev bot for updating the code. If you encounter any bugs or issues with the patch, please report them here.

Ask @zeropath-ai-dev!

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

@zeropath-ai-dev 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_1737105809757082

# if vscode is installed run (or use your favorite editor / IDE):
code superagi/controllers/resources.py

# 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_1737105809757082

@r0path r0path merged commit 6cd85ef into main Jan 17, 2025
1 check passed
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