Skip to content

Conversation

@yoff
Copy link
Contributor

@yoff yoff commented Nov 26, 2025

Copilot AI review requested due to automatic review settings November 26, 2025 12:37
@yoff yoff requested a review from a team as a code owner November 26, 2025 12:37
@yoff yoff changed the title Python/header splitting experiments Python: detecting header splitting in synthetic app Nov 26, 2025
Copilot finished reviewing on behalf of yoff November 26, 2025 12:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds test coverage and modeling for header injection vulnerabilities using Python's http.server module and extends support for urllib.parse.ParseResult.

Key changes:

  • Adds a test case demonstrating header injection vulnerability in http.server.BaseHTTPRequestHandler
  • Introduces models for urllib.parse.ParseResult class to track taint propagation through URL parsing
  • Extends urllib.parse.urlparse function modeling with taint propagation

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
python/ql/test/query-tests/Security/CWE-113-HeaderInjection/Tests1/http_test.py New test case demonstrating header injection vulnerability using http.server module
python/ql/test/query-tests/Security/CWE-113-HeaderInjection/Tests1/HeaderInjection.expected Updated expected test results to include new test case findings (generated file)
python/ql/lib/semmle/python/frameworks/Stdlib.qll Adds ParseResult module with taint tracking for urllib.parse.ParseResult class
python/ql/lib/semmle/python/frameworks/Stdlib.model.yml Adds taint model for urlparse function and type model for ParseResult class

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +19 to +22
# if __name__ == "__main__":
# print("Serving vulnerable app on http://127.0.0.1:8080")
# httpd = HTTPServer(("127.0.0.1", 8080), VulnerableHandler)
# httpd.serve_forever()
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment appears to contain commented-out code.

Suggested change
# if __name__ == "__main__":
# print("Serving vulnerable app on http://127.0.0.1:8080")
# httpd = HTTPServer(("127.0.0.1", 8080), VulnerableHandler)
# httpd.serve_forever()
# To run this server manually for testing, use:
# python -m http.server 8080 --bind 127.0.0.1
# Or implement the main block as shown in documentation.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally leave such a code block there, so people can easily test for actual vulnerabilities.

@@ -0,0 +1,22 @@
from http.server import HTTPServer, BaseHTTPRequestHandler
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import of 'HTTPServer' is not used.

Suggested change
from http.server import HTTPServer, BaseHTTPRequestHandler
from http.server import BaseHTTPRequestHandler

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@joefarebrother joefarebrother left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants