Validate Alternate Repository Location URLS #16817
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| project_name=self.project.name, | ||
| ) | ||
| ) | ||
| return self.manage_project_settings(add_alternate_repository_form=form) |
Check warning
Code scanning / CodeQL
Reflected server-side cross-site scripting Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 1 month ago
To fix the reflected server-side cross-site scripting vulnerability, we need to ensure that any user-provided data is properly escaped before being rendered in the template. In this case, we can use the html.escape() function from Python's standard library to escape the form data before it is included in the dictionary returned by the manage_project_settings method.
| @@ -1161,2 +1161,3 @@ | ||
|
|
||
| import html | ||
| return { | ||
| @@ -1170,3 +1171,3 @@ | ||
| ), | ||
| "add_alternate_repository_form_class": add_alt_repo_form, | ||
| "add_alternate_repository_form_class": html.escape(str(add_alt_repo_form)), | ||
| } |
| @@ -14,6 +14,8 @@ | |||
|
|
|||
| import wtforms | |||
|
|
|||
| from urllib3 import PoolManager, Timeout | |||
There was a problem hiding this comment.
question: we use requests in other places in the code (example), and this appears to be the first time we're using urllib3 directly. Is that to get both connect and read timeouts? If so, then we can pass those as a tuple to requests like so:
response = requests.head(field.data, timeout=(1.0, 1.0), headers=...)
But that apparently has the same affect as only specifying it once, since th value will be applied to both, so we could satisfy with
response = requests.head(field.data, timeout=1.0, headers=...)
https://requests.readthedocs.io/en/latest/user/advanced/#timeouts
I'm also wondering why we aren't using request.http.head(...) from
Lines 48 to 53 in 1fbb4ac
request object available to them?
| ) | ||
| except Exception as exc: | ||
| raise wtforms.validators.ValidationError( | ||
| _(f"Unable to parse simple index at given url: {exc}") |
There was a problem hiding this comment.
| _(f"Unable to parse simple index at given url: {exc}") | |
| _(f"The URL provided does not respond as a Simple repository: {exc}") |
And maybe even tack on a URL? https://packaging.python.org/specifications/simple-repository-api/
| @@ -1129,7 +1129,7 @@ def __init__(self, project, request): | |||
| self.add_alternate_repository_form_class = AddAlternateRepositoryForm | |||
|
|
|||
| @view_config(request_method="GET") | |||
| def manage_project_settings(self): | |||
| def manage_project_settings(self, add_alternate_repository_form=None): | |||
There was a problem hiding this comment.
question: is this because we're using two methods for GET vs POST, and how to make sure that the form values aren't reset on validation failure? I don't know if I've seen this pattern before, and wanted to understand it more.
User behavior for this configuration has shown that most URLs supplied are not simple indexes at all, predominately they have been source repository links.
This is a bare-bones approach to validating that the URL is actually a simple index.