From 44c8450c99a6bf8504bc44750da919743d9c051c Mon Sep 17 00:00:00 2001 From: Postmodern Date: Fri, 24 May 2024 23:55:07 -0700 Subject: [PATCH] Simplify the `OpenRedirect` regexs (closes #77). * 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. --- lib/ronin/vulns/open_redirect.rb | 16 ++-- spec/open_redirect_spec.rb | 159 +++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 8 deletions(-) diff --git a/lib/ronin/vulns/open_redirect.rb b/lib/ronin/vulns/open_redirect.rb index 598e7d9..9a6a0fe 100644 --- a/lib/ronin/vulns/open_redirect.rb +++ b/lib/ronin/vulns/open_redirect.rb @@ -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 @@ -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* diff --git a/spec/open_redirect_spec.rb b/spec/open_redirect_spec.rb index b1aaf0b..b344eff 100644 --- a/spec/open_redirect_spec.rb +++ b/spec/open_redirect_spec.rb @@ -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/" } @@ -499,6 +511,27 @@ end end end + + context "and the url value has an additional appended string" do + let(:response_body) do + <<~HTML + + + + + +

example content

+

included content

+

more content

+ + + 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 @@ -583,6 +616,27 @@ end end end + + context "and the url value has an additional appended string" do + let(:response_body) do + <<~HTML + + + + + +

example content

+

included content

+

more content

+ + + HTML + end + + it "must return true" do + expect(subject.vulnerable?).to be_truthy + end + end end end @@ -669,6 +723,27 @@ end end end + + context "and the url value has an additional appended string" do + let(:response_body) do + <<~HTML + + + + + +

example content

+

included content

+

more content

+ + + 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 @@ -753,6 +828,27 @@ end end end + + context "and the url value has an additional appended string" do + let(:response_body) do + <<~HTML + + + + + +

example content

+

included content

+

more content

+ + + HTML + end + + it "must return true" do + expect(subject.vulnerable?).to be_truthy + end + end end end @@ -839,6 +935,27 @@ end end end + + context "and the url value has an additional appended string" do + let(:response_body) do + <<~HTML + + + + + +

example content

+

included content

+

more content

+ + + 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 @@ -923,6 +1040,27 @@ end end end + + context "and the url value has an additional appended string" do + let(:response_body) do + <<~HTML + + + + + +

example content

+

included content

+

more content

+ + + 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 @@ -1007,6 +1145,27 @@ end end end + + context "and the url value has an additional appended string" do + let(:response_body) do + <<~HTML + + + + + +

example content

+

included content

+

more content

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