Fixed rebuild_safe_url function

[gojo-satorou-v7]

User description

Now the rebuild_safe_url checks for potential ssrf payload and returns None in case such url is provided, also I have made the path sanitization better since the previous one only removed the query and fragment part from the path but now it checks furtherfor carriage return and line feed and other misused paths and normalizes them.

fixes #3797
fixes #3923

---

PR Type

Bug fix, Tests

---

Description

• Enhanced rebuild_safe_url to sanitize URLs against SSRF vulnerabilities.

• Added is_dns_safe function to validate DNS safety of URLs.

• Introduced comprehensive test cases for rebuild_safe_url function.

• Removed deprecated is_safe_url function to streamline URL validation.

---

Changes walkthrough 📝

	Relevant files
Tests	test_api.py Add test cases for `rebuild_safe_url` function                      website/test_api.py Added a new test case class RebuildSafeUrlTestCase. Included multiple test scenarios for rebuild_safe_url. Validated URL sanitization against encoded characters, CRLF, and path traversal. +28/-0
Bug fix	utils.py Improve URL sanitization and add DNS safety checks              website/utils.py Enhanced rebuild_safe_url to sanitize URLs against SSRF vulnerabilities. Added is_dns_safe function to validate DNS safety of hostnames. Improved path normalization and encoding in rebuild_safe_url. Removed deprecated is_safe_url function. +53/-17

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 0ae056d)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 3797 - Partially compliant Compliant requirements: Ensure rebuild_safe_url function sanitizes URLs to prevent SSRF vulnerabilities. Validate DNS safety of URLs to block internal, loopback, and other unsafe addresses. Normalize and sanitize paths to prevent misuse (e.g., CRLF injection, path traversal). Remove deprecated or redundant functions related to URL validation. Non-compliant requirements: [] Requires further human verification: []

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The path.replace logic in rebuild_safe_url might not handle all edge cases of path traversal or CRLF injection. Ensure comprehensive testing and validation. path = parsed_url.path path = path.replace("\r", "").replace("\n", "") path = path.replace("/..", "").replace("/", "/") # Collapse multiple slashes into a single slash path = re.sub(r"/{2,}", "/", path) if path in ("", "."): path = "/" # Ensure the path starts with a slash elif not path.startswith("/"): path = "/" + path encoded_path = quote(path, safe="/") DNS Safety Validation The is_dns_safe function might not account for all edge cases of unsafe DNS resolution. Verify its robustness against potential bypass techniques. def is_dns_safe(hostname): try: resolved = socket.getaddrinfo(hostname, None) except socket.gaierror: return False # Unable to resolve hostname; treat as unsafe. for result in resolved: ip_str = result[4][0] try: ip = ip_address(ip_str) if ip.is_private or ip.is_loopback or ip.is_reserved or ip.is_link_local: return False except ValueError: continue return True

[github-actions]

PR Code Suggestions ✨

[github-actions]

Persistent review updated to latest commit 0ae056d

[github-actions]

PR Code Suggestions ✨

[gojo-satorou-v7]

This fixes the issue for the rebuild_safe_url which was introducing SSRF vulnerabilities in the /website/views/issue.py at file issues.py and company.py by improperly sanitizing the url. My changes to the code bring enhanced protection from SSRF, CRLF and path traversal vulnerabilities by properly sanitizing and blocking requests to internal ip address.

The dns_safe is another function added which first resolves the input url from the user and then resolves it's ip address before processing it any further [Protects from dns rebinding] . It is advisable to use this version of rebuild_safe_url wherever there;s a need to take user input.

[gojo-satorou-v7]

@DonnieBLT I have added the test case, please check if any improvements are needed or edge case I might've missed.

[gojo-satorou-v7]

Since the fix for #3923 was small I have included it in this PR.
fixes #3923

[gojo-satorou-v7]

Can this be merged? @DonnieBLT