Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for end of string when parsing Gopher URLs #218

Merged
merged 1 commit into from
Feb 19, 2022
Merged

Conversation

rkta
Copy link
Contributor

@rkta rkta commented Feb 17, 2022

This fixes issue #199 reported by Kuang-che Wu.

A specially crafted Gopher URL (e.g. '') could lead to
an out-of-bounds read.

Problem here was, that 'p' was incremented twice without checking for
the end of the string.

The interesting question for me is: What does this 'if' actually check?
What is special here about the 'R'? I did not find anything related in
RFC 1436 or in RFC 4266.

This fixes issue tats#199 reported by Kuang-che Wu.

A specially crafted Gopher URL (e.g. '<a href=gopher:R>') could lead to
an out-of-bounds read.

Problem here was, that 'p' was incremented twice without checking for
the end of the string.

The interesting question for me is: What does this 'if' actually check?
What is special here about the 'R'? I did not find anything related in
RFC 1436 or in RFC 4266.
@tats
Copy link
Owner

tats commented Feb 17, 2022

Though I don't know about the 'R', it's enough to fix the issue,
so acceptable for now.

@rkta
Copy link
Contributor Author

rkta commented Feb 17, 2022 via email

@rkta rkta marked this pull request as ready for review February 17, 2022 20:52
@rkta
Copy link
Contributor Author

rkta commented Feb 17, 2022

Sorry, I just realized this was a draft, this was not was I intended.

@tats tats merged commit 32c1ad1 into tats:master Feb 19, 2022
@tats
Copy link
Owner

tats commented Feb 19, 2022

Merged, thanks for your contribution.

@rkta rkta deleted the issue199 branch May 1, 2022 09:46
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.

None yet

2 participants