python: model send_header from http.server
#19432
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR models the send_header method from http.server for CodeQL analysis by updating tests and adding a new QL class.
- Adds CodeQL taint annotations to the HTTP server test file.
- Introduces HeaderWriteCall in the Stdlib QL library to capture header write calls.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/ql/test/library-tests/frameworks/stdlib/http_server.py | Added CodeQL annotations to the send_header call to mark unsanitized header write details. |
| python/ql/lib/semmle/python/frameworks/Stdlib.qll | Added a new class, HeaderWriteCall, to model calls to send_header including associated taint handling. |
| override DataFlow::Node getValueArg() { result = this.getArg(1) } | ||
|
|
||
| // TODO: These checks perhaps could be made more precise. | ||
| override predicate nameAllowsNewline() { any() } |
There was a problem hiding this comment.
[nitpick] Consider refining the predicate for nameAllowsNewline if possible, rather than using a blanket 'any()' which may reduce the precision of the taint tracking.
| override predicate nameAllowsNewline() { any() } | |
| override predicate nameAllowsNewline() { not exists(this.getNameArg().toString().regexpMatch(".*[\n\r].*")) } |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
It is not about the argument itself, but rather wether the server configuration would allow a newline to appear here. http.server does not have any protection against this, so any is correct.
| // TODO: These checks perhaps could be made more precise. | ||
| override predicate nameAllowsNewline() { any() } | ||
|
|
||
| override predicate valueAllowsNewline() { any() } |
There was a problem hiding this comment.
[nitpick] Consider refining the predicate for valueAllowsNewline if feasible, to improve the accuracy of tracking header values in the taint analysis.
| override predicate valueAllowsNewline() { any() } | |
| override predicate valueAllowsNewline() { | |
| not exists(DataFlow::Node value | value = this.getValueArg() | | |
| value.toString().matches("%0A") or value.toString().matches("%0D") | |
| ) | |
| } |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Same as for the name argument above.
Requested here