Skip to content

Commit e4d4fc3

Browse files
committed
Unescape and resolve paths before resource checks
Fix scenario where someone could use '../' to access private resources
1 parent 145a5df commit e4d4fc3

File tree

5 files changed

+29
-10
lines changed

5 files changed

+29
-10
lines changed

Diff for: CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
# Change Log
22
All notable changes to this project will be documented in this file.
33

4+
## 1.0.4 - 2019-11-13
5+
### Security
6+
- Escape and resolve path before evaluating resource rules (thanks to Colby Morgan)
7+
48
## 1.0.3 - 2019-03-24
59
### Changed
610
- Don't send 'Content-Type' header with pre-flight requests

Diff for: lib/rack/cors.rb

+17-9
Original file line numberDiff line numberDiff line change
@@ -64,24 +64,27 @@ def allow(&block)
6464
def call(env)
6565
env[HTTP_ORIGIN] ||= env[HTTP_X_ORIGIN] if env[HTTP_X_ORIGIN]
6666

67+
path = evaluate_path(env)
68+
6769
add_headers = nil
6870
if env[HTTP_ORIGIN]
6971
debug(env) do
7072
[ 'Incoming Headers:',
7173
" Origin: #{env[HTTP_ORIGIN]}",
74+
" Path-Info: #{path}",
7275
" Access-Control-Request-Method: #{env[HTTP_ACCESS_CONTROL_REQUEST_METHOD]}",
7376
" Access-Control-Request-Headers: #{env[HTTP_ACCESS_CONTROL_REQUEST_HEADERS]}"
7477
].join("\n")
7578
end
7679
if env[REQUEST_METHOD] == OPTIONS and env[HTTP_ACCESS_CONTROL_REQUEST_METHOD]
77-
headers = process_preflight(env)
80+
headers = process_preflight(env, path)
7881
debug(env) do
7982
"Preflight Headers:\n" +
8083
headers.collect{|kv| " #{kv.join(': ')}"}.join("\n")
8184
end
8285
return [200, headers, []]
8386
else
84-
add_headers = process_cors(env)
87+
add_headers = process_cors(env, path)
8588
end
8689
else
8790
Result.miss(env, Result::MISS_NO_ORIGIN)
@@ -90,7 +93,7 @@ def call(env)
9093
# This call must be done BEFORE calling the app because for some reason
9194
# env[PATH_INFO] gets changed after that and it won't match. (At least
9295
# in rails 4.1.6)
93-
vary_resource = resource_for_path(env[PATH_INFO])
96+
vary_resource = resource_for_path(path)
9497

9598
status, headers, body = @app.call env
9699

@@ -147,14 +150,20 @@ def select_logger(env)
147150
end
148151
end
149152

153+
def evaluate_path(env)
154+
path = env[PATH_INFO]
155+
path = Rack::Utils.clean_path_info(Rack::Utils.unescape_path(path)) if path
156+
path
157+
end
158+
150159
def all_resources
151160
@all_resources ||= []
152161
end
153162

154-
def process_preflight(env)
163+
def process_preflight(env, path)
155164
result = Result.preflight(env)
156165

157-
resource, error = match_resource(env)
166+
resource, error = match_resource(path, env)
158167
unless resource
159168
result.miss(error)
160169
return {}
@@ -163,8 +172,8 @@ def process_preflight(env)
163172
return resource.process_preflight(env, result)
164173
end
165174

166-
def process_cors(env)
167-
resource, error = match_resource(env)
175+
def process_cors(env, path)
176+
resource, error = match_resource(path, env)
168177
if resource
169178
Result.hit(env)
170179
cors = resource.to_headers(env)
@@ -185,8 +194,7 @@ def resource_for_path(path_info)
185194
nil
186195
end
187196

188-
def match_resource(env)
189-
path = env[PATH_INFO]
197+
def match_resource(path, env)
190198
origin = env[HTTP_ORIGIN]
191199

192200
origin_matched = false

Diff for: lib/rack/cors/version.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
module Rack
22
class Cors
3-
VERSION = "1.0.3"
3+
VERSION = "1.0.4"
44
end
55
end

Diff for: test/unit/cors_test.rb

+6
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,12 @@ def load_app(name, options = {})
146146
last_response.headers['Vary'].must_equal 'Origin, Host'
147147
end
148148

149+
it "decode URL and resolve paths before resource matching" do
150+
header 'Origin', 'http://localhost:3000'
151+
get '/public/a/..%2F..%2Fprivate/stuff'
152+
last_response.wont_render_cors_success
153+
end
154+
149155
describe 'with array of upstream Vary headers' do
150156
let(:app) { load_app('test', { proxy: true }) }
151157

Diff for: test/unit/test.ru

+1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ use Rack::Cors do
4141
allow do
4242
origins '*'
4343
resource '/public'
44+
resource '/public/*'
4445
resource '/public_without_credentials', :credentials => false
4546
end
4647

0 commit comments

Comments
 (0)