Skip to content

Conversation

DilumAluthge
Copy link
Member

This is a follow-up to #50.

#50 disallowed some path navigation (specifically the pattern ..).

In this PR, we also disallow some additional path traversal patterns (such as ./) that are not covered by #50.

@DilumAluthge
Copy link
Member Author

cc: @tanmaykm

@@ -187,10 +187,19 @@ struct Endpoint
query::Dict=Dict(),
allow_404::Bool=false,
)
# do not allow path navigation in URLs
# Do not allow path navigation in URLs
# Disallowed pattern: ..
if occursin(r"\.\.", url)
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if this is needed any more... the specific path traversal cases it was needed for preventing seem to be covered in the below regex. Should this be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm. Technically the regex PATH_TRAVERSAL doesn't actually cover the pattern .. - is that okay?

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see that it does not cover r"^\.\.$", which is a valid path traversal without /. But that seems to be the only one, right? Should we then change it to that?

Copy link
Member

Choose a reason for hiding this comment

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

But then, an Endpoint without / is not really valid. So it does seem okay to remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants