Comments (10)
Yeah, this patch needs to go in. @sporkmonger did you look at this?
from addressable.
I did. It hasn't been ignored, I just hadn't gotten around to explaining why I was rejecting it yet.
RFC 3986 has a very specific algorithm for dot segment removal that's given as part of the spec in section 5.2.4. That algorithm is what Addressable uses for dot segment resolution. I'm not inclined to deviate from it or the related algorithms described in section 5.2.2 and section 5.2.3.
http://www.example.com/
and http://www.example.com//
are not equivalent and should not be treated as if they are. Even browsers don't strip double-slashes. Addressable does not engage in magic. It implements a spec as precisely as possible with only a few exceptions. I'm not inclined to make this one of them. Additionally, I fail to see how this is a security issue outside of developers who are misusing the tool and failing to take the standard precautions that should be being put into place in any application that deals with relative references.
If this is a problem for you, you are welcome to fork.
from addressable.
Though I'll reopen briefly because normalization on that last example probably should not have returned http://www.example.com///../
.
from addressable.
I'm not interested and/or sufficiently knowledgeable on this topic to really maintain another fork of what is already a very solid piece of code, however I do think that last example merits some investigation and possibly a fix. I agree and already understand the low security potential of this to people that use better security mechanisms, but I still think this qualifies as a security issue, low-risk as it may be (for well designed apps).
Edit: apologies, didn't see your followup just now.
from addressable.
Yeah, take a look at the associated patch to see what I'm really objecting to.
from addressable.
Thanks for the links. You are correct regarding my patch being incorrect. I wasn't aware of the behaviour of the multiple slashes in the RFC.
As for the security concern, I thought it was a reasonable assumption that someone could (incorrectly) rely on the normalisation to prevent directory traversal attacks.
from addressable.
Well, it should prevent traversal above root:
require 'addressable/uri'
puts Addressable::URI.join('http://www.example.com/', '..')
puts Addressable::URI.join('http://www.example.com/', '/..')
puts Addressable::URI.join('http://www.example.com/', '/../')
Output:
http://www.example.com/
http://www.example.com/
http://www.example.com/
That's the main threat to be concerned about in dot segment resolution.
from addressable.
Yeah, except if the second parameter is user controlled then it could match my last example.
The file system does not treat // as multiple folders so my last example would traverse above the root.
Any idea how far off a patch will be for this issue?
from addressable.
If the second parameter is user-controlled, you can't afford to skip a separate application-level security check to verify you haven't traversed above your web root or whatever sandbox the user should remain within.
from addressable.
Yeah, I totally agree. I was talking about someone incorrectly using Addressable for the application-level security check though.
from addressable.
Related Issues (20)
- Fuzz test for parse/to_s round-trip
- Day HOT 5
- Dependabot couldn't find a Gemfile HOT 3
- Templates doesn't handle IPv6 IPs
- Invalid scheme format for ssh URL HOT 4
- Is it intended that `normalized_path` destroys the trailing dot when it's the only char? HOT 1
- Improve pure ruby IDNA implementation to match browsers behavior (IDNA2008 and UTS#46) HOT 3
- Equivalent of `URI.regexp(schemes)`? HOT 4
- Crypto mining
- undefined method `to_str' for :id:Symbol (NoMethodError) in 2.8.2 HOT 8
- Template expansion does not work with symbolized hashes in 2.8.1 HOT 1
- Update to 2.8.2 break test env HOT 1
- Any version after 2.8.1 causes errors in our test suite coming from addressable. HOT 8
- Drop support for Ruby 2.2 (and more?) HOT 3
- Disallow backtick in host HOT 1
- Normalize errors when trying to run a simple url normalize HOT 4
- Unsafe concurrent Hash access HOT 9
- k
- feed:http: crashes servers HOT 11
- Valid domain not parsing HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from addressable.