Skip to content

Commit

Permalink
Simplify the OpenRedirect regexs (closes #77).
Browse files Browse the repository at this point in the history 
* Detect when the test URL has an additional string appended to it
  (ex: `.html`). The appended string can easily be bypassed by adding a
  `?`, `&`, or `#` character to the end of the test URL.
* Added more test cases.
  • Loading branch information
@postmodern
postmodern committed May 25, 2024
1 parent 5a95758 commit 44c8450
Show file tree
Hide file tree
Showing 2 changed files with 167 additions and 8 deletions.
16 changes: 8 additions & 8 deletions lib/ronin/vulns/open_redirect.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def vulnerable?
when '301', '302', '303', '307', '308'
if (locations = response.get_fields('Location'))
escaped_test_url = Regexp.escape(@test_url)
regexp = /\A#{escaped_test_url}(?:[\?&].+)?\z/
regexp = /\A#{escaped_test_url}.*\z/

locations.last =~ regexp
end
Expand All @@ -99,26 +99,26 @@ def vulnerable?
"\s*\d+\s*;\s*url\s*=\s*
(?:
# content="0; url='...'"
'\s*#{escaped_test_url}(?:(?:\?|&(amp;)?)[^\s'"]+)?\s*' |
'\s*#{escaped_test_url}[^'"]*' |
# content="0; url=..."
#{escaped_test_url}(?:(?:\?|&(amp;)?)[^\s"]+)?
#{escaped_test_url}[^"]*
)\s*" |
# content='...'
'\s*\d+\s*;\s*url\s*=\s*
(?:
# content='0; url="..."'
"\s*#{escaped_test_url}(?:(?:\?|&(amp;)?)[^\s"']+)?\s*" |
"\s*#{escaped_test_url}[^"']*" |
# content='0; url=...'
#{escaped_test_url}(?:(?:\?|&(amp;)?)[^\s']+)?
#{escaped_test_url}[^']*
)\s*' |
# content=...
\s*\d+;url=(?:
# content=0;url="..."
"\s*#{escaped_test_url}(?:(?:\?|&(amp;)?)[^\s"]+)?\s*" |
"\s*#{escaped_test_url}[^\s"]*" |
# content=0;url='...'
'\s*#{escaped_test_url}(?:(?:\?|&(amp;)?)[^\s']+)?\s*' |
'\s*#{escaped_test_url}[^\s']*' |
# content=0;url=...
#{escaped_test_url}(?:(?:\?|&(amp;)?)[^\s/>]+)?
#{escaped_test_url}[^\s/>]*
)
)
\s*
Expand Down
159 changes: 159 additions & 0 deletions spec/open_redirect_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,18 @@
end
end

context "and it starts with #test_url but with an additional appended string" do
let(:location) { "#{subject.test_url}.html" }

before do
allow(response).to receive(:get_fields).with('Location').and_return([location])
end

it "must return true" do
expect(subject.vulnerable?).to be_truthy
end
end

context "but it does not match #test_url" do
let(:location) { "https://example.com/" }

Expand Down Expand Up @@ -499,6 +511,27 @@
end
end
end

context "and the url value has an additional appended string" do
let(:response_body) do
<<~HTML
<html>
<head>
<meta http-equiv="refresh" content='0;url="#{subject.test_url}.html"'/>
</head>
<body>
<p>example content</p>
<p>included content</p>
<p>more content</p>
</body>
</html>
HTML
end

it "must return true" do
expect(subject.vulnerable?).to be_truthy
end
end
end

context "and the url value is not quoted" do
Expand Down Expand Up @@ -583,6 +616,27 @@
end
end
end

context "and the url value has an additional appended string" do
let(:response_body) do
<<~HTML
<html>
<head>
<meta http-equiv="refresh" content='0;url=#{subject.test_url}.html'/>
</head>
<body>
<p>example content</p>
<p>included content</p>
<p>more content</p>
</body>
</html>
HTML
end

it "must return true" do
expect(subject.vulnerable?).to be_truthy
end
end
end
end

Expand Down Expand Up @@ -669,6 +723,27 @@
end
end
end

context "and the url value has an additional appended string" do
let(:response_body) do
<<~HTML
<html>
<head>
<meta http-equiv="refresh" content="0;url='#{subject.test_url}.html'"/>
</head>
<body>
<p>example content</p>
<p>included content</p>
<p>more content</p>
</body>
</html>
HTML
end

it "must return true" do
expect(subject.vulnerable?).to be_truthy
end
end
end

context "and the url value is not quoted" do
Expand Down Expand Up @@ -753,6 +828,27 @@
end
end
end

context "and the url value has an additional appended string" do
let(:response_body) do
<<~HTML
<html>
<head>
<meta http-equiv="refresh" content="0;url=#{subject.test_url}.html"/>
</head>
<body>
<p>example content</p>
<p>included content</p>
<p>more content</p>
</body>
</html>
HTML
end

it "must return true" do
expect(subject.vulnerable?).to be_truthy
end
end
end
end

Expand Down Expand Up @@ -839,6 +935,27 @@
end
end
end

context "and the url value has an additional appended string" do
let(:response_body) do
<<~HTML
<html>
<head>
<meta http-equiv="refresh" content=0;url="#{subject.test_url}.html"/>
</head>
<body>
<p>example content</p>
<p>included content</p>
<p>more content</p>
</body>
</html>
HTML
end

it "must return true" do
expect(subject.vulnerable?).to be_truthy
end
end
end

context "and the url value is single quoted" do
Expand Down Expand Up @@ -923,6 +1040,27 @@
end
end
end

context "and the url value has an additional appended string" do
let(:response_body) do
<<~HTML
<html>
<head>
<meta http-equiv="refresh" content=0;url='#{subject.test_url}.html'/>
</head>
<body>
<p>example content</p>
<p>included content</p>
<p>more content</p>
</body>
</html>
HTML
end

it "must return true" do
expect(subject.vulnerable?).to be_truthy
end
end
end

context "and the url value is not quoted" do
Expand Down Expand Up @@ -1007,6 +1145,27 @@
end
end
end

context "and the url value has an additional appended string" do
let(:response_body) do
<<~HTML
<html>
<head>
<meta http-equiv="refresh" content=0;url=#{subject.test_url}.html/>
</head>
<body>
<p>example content</p>
<p>included content</p>
<p>more content</p>
</body>
</html>
HTML
end

it "must return true" do
expect(subject.vulnerable?).to be_truthy
end
end
end
end

Expand Down

0 comments on commit 44c8450

Please sign in to comment.