fix(parser): reject whitespace in the URL token - #1379
Conversation
8c9c102 to
fac695e
Compare
r266-tech
left a comment
There was a problem hiding this comment.
Please add a regression test for the reported injection shape, for example name @ https://host/file.whl\nevil==1, and assert that constructing Requirement raises InvalidRequirement. The current coverage only exercises trailing line breaks, so it does not directly preserve the security-relevant invariant that attacker-controlled text after embedded URL whitespace cannot be parsed and later serialized as another dependency line.
The checked-out head defines the URL tokenizer rule as r"\S+". The authority verification confirms that the checkout matches head fac695e96a4235f82bd4e67b875704025f3b6dc1, and git diff --check exited 0. The requested pytest checks were not run because the authority environment's system Python has no pytest and cannot create its cache file.
|
Added the injection-shape test: |
The URL token in the dependency-specifier tokenizer matches
[^ \t]+, so it stops at spaces and tabs but keeps going through a newline. A direct-reference requirement likefoo @ https://host/foo.whl\nevil==1parses without complaint, with the trailing newline andevil==1folded intourl. Callingstr()on that requirement then prints two lines, so any tool that writes parsed requirements one per line (a requirements file, a generated lockfile) picks up the injected second dependency.Match the URL token with
\S+instead, so it ends at any whitespace, in line with RFC 3986 having no whitespace in a URI. The name, extras, and specifier tokens already reject newlines, and marker values go throughast.literal_eval, so this just brings the URL branch up to the same footing. Valid URLs never carry whitespace, so parsing of real direct references is unchanged.