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

fix (backend): Patching the SSRF vulnerability in Github/Web Search/Request related blocks #8531

Merged
merged 18 commits into from
Nov 8, 2024

Conversation

jackfromeast
Copy link
Contributor

Background

Refer to the report on huntr.com: https://huntr.com/bounties/1d91e1e1-7d45-4bda-bc27-bfe9052fd975

Changes 🏗️

  • Add the domain validation (i.e., github.com) to all the blocks under /blocks/github
  • Add the URL validation to the Web Search block and Web Request block: Block all the private IP requests by default unless listed in the config file's trust_endpoints_for_requests field.

Testing 🔍

Tested locally.

Configuration Changes 📝

  • Add a trust_endpoints_for_requests field for the developer to create whitelist domains to request.

@jackfromeast jackfromeast requested a review from a team as a code owner November 3, 2024 20:13
@jackfromeast jackfromeast requested review from Swiftyos and kcze and removed request for a team November 3, 2024 20:13
@CLAassistant
Copy link

CLAassistant commented Nov 3, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added platform/backend AutoGPT Platform - Back end platform/blocks labels Nov 3, 2024
Copy link
Contributor

github-actions bot commented Nov 3, 2024

This PR targets the master branch but does not come from dev or a hotfix/* branch.

Automatically setting the base branch to dev.

@github-actions github-actions bot changed the base branch from master to dev November 3, 2024 20:14
Copy link

qodo-merge-pro bot commented Nov 3, 2024

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns

SSRF protection:
The PR introduces URL validation to prevent Server-Side Request Forgery (SSRF) attacks. This is a significant security improvement, but the implementation should be carefully reviewed to ensure it covers all possible scenarios and edge cases. Pay special attention to the validate_url functions in http.py and search.py, and make sure they are consistent and comprehensive in their checks.

⚡ Recommended focus areas for review

Security Concern
The validate_url function is a critical security measure. Ensure it correctly prevents SSRF attacks and properly handles all edge cases.

Redundant Code
The is_github_url function is defined multiple times across different files. Consider moving it to a shared utility module to improve maintainability.

Code Duplication
The validate_url method in the GetRequest class is nearly identical to the one in the http.py file. Consider refactoring to use a shared implementation.

@github-actions github-actions bot added the size/l label Nov 3, 2024
Copy link

netlify bot commented Nov 3, 2024

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 86c3f50
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/672d593faea3d80008003aa2

@ntindle ntindle requested a review from majdyz November 4, 2024 12:48
@github-actions github-actions bot added size/xl and removed size/l labels Nov 6, 2024
@github-actions github-actions bot added the platform/frontend AutoGPT Platform - Front end label Nov 6, 2024
@majdyz majdyz requested a review from ntindle November 6, 2024 08:49
@majdyz majdyz requested review from ntindle and majdyz November 7, 2024 22:57
Copy link
Contributor

github-actions bot commented Nov 7, 2024

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Nov 7, 2024
Copy link
Contributor

github-actions bot commented Nov 8, 2024

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Nov 8, 2024
@majdyz majdyz enabled auto-merge (squash) November 8, 2024 00:22
@majdyz majdyz merged commit bcaf324 into Significant-Gravitas:dev Nov 8, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants