enhanced path validation in Windows #1379

orangetw · 2018-01-09T04:51:40Z

Due to the Windows environment, more check backslashes in static! method and enhanced the path_traversal validation in rack-protection

kgrz · 2018-01-13T06:36:07Z

lib/sinatra/base.rb

@@ -1061,6 +1061,7 @@ def route_missing
    def static!(options = {})
      return if (public_dir = settings.public_folder).nil?
      path = File.expand_path("#{public_dir}#{URI_INSTANCE.unescape(request.path_info)}" )
+      return unless path.start_with?(public_dir)


Can you add a comment about when this might be true/false?

Oh, sorry I misunderstood your question :(
This line check the user-supplied path must under public_dir, or it will return nil to prevent user from path traversal attack.

@kgrz Did you mean "Can you add a Ruby code comment near this line about ..."?

Hehe, yes yes. I meant a comment in the code. Didn’t notice my question was ambiguous until you pointed it out.

I agree a code comment about when path.start_with?(public_dir) would evaluate to false would be helpful. I'm actually not sure I understand when that would happen 😬

I think this is not necessary actually because . and .. have been reduced by rack-protection.
Even in the case of invalidating the :path_traversal setting on rack-protection, you should still add comment in the code if you think that it is meaningful to activate this restriction.
@orangetw Could you add the comment in the code?

The major enhancement I already commit to rack-protections (added back-slash check to the path_traversal.rb), this line(in base.rb) is just double check and ensure that there is no unknown(unexpected) attack vector.

I am not a ruby developer, so if you think that line is unnecessary, maybe only merge the changes on path_traversal.rb?

As a personal opinion, I prefer a simple structure.
Double check increases software complexity.
And yes, the essence of the problem is certainly fixed with your commit to rack-protection.
If you agree with my opinion, please drop this line.

OK, I have done that!

Great, I'm going to check the behavior on Windows and merge this in a few days.
Thank you!

zzak · 2018-01-14T03:52:41Z

cc #1339

namusyaka · 2018-02-10T17:56:47Z

Btw, thank you for sending the patch and your report!

Removed the double check line!

namusyaka

Looks good to me. Please wait for confirming this change on my windows machine.

namusyaka · 2018-02-11T06:49:15Z

I've confirmed the issue, and it has been fixed correctly by this patch.
@orangetw Thank you so much!

This commit has been backported from sinatra/sinatra#1379 Fixes CVE-2018-7212

peterkeen · 2018-02-27T21:21:21Z

Hi folks, does this affect versions < 2.0?

andrew-stripe · 2018-02-27T21:25:24Z

@peterkeen: I was just looking into the same thing myself. I believe it effects all versions of rack-protection before 2.0.1. I think it would be possible to backport the fix here and cut a 1.5.4 release from the separate rack-protection repo:
https://github.com/sinatra/rack-protection/blob/v1.5.3/lib/rack/protection/path_traversal.rb

andrew-stripe · 2018-02-27T22:13:52Z

Ah, actually this was already backported:
https://github.com/sinatra/rack-protection/commits/stable-1.5

So this really is fixed on rack-protection ~>1.5.4 / >2.0.0 I think

ghiculescu · 2018-02-27T22:43:16Z

If bundle-audit told you to come here and you don't want to update to Sinatra 2.x right now, subscribe to rubysec/ruby-advisory-db#331

namusyaka · 2018-02-28T01:03:24Z

Yeah, rack-protection v1.5.x has not been maintained, I had backported security fix exceptionally into the 1.5-stable branch.

nokogiri 1.8.2 fixes security issue https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15412 rack-protection 2.0.1 fixes security issue sinatra/sinatra#1379

update Sinatra due to security advisory: Name: rack-protection Version: 1.5.3 Advisory: CVE-2018-7212 Criticality: Unknown URL: sinatra/sinatra#1379 Title: Path traversal is possible via backslash characters on Windows. Solution: upgrade to >= 2.0.1, ~> 1.5.4

nokogiri 1.8.2 fixes security issue https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15412 rack-protection 2.0.1 fixes security issue sinatra/sinatra#1379

Addresses CVE-2018-7212 reported in rack-protection.[1] CVE does not affect Supermarket because the vulnerability is only on Windows platforms. [1] sinatra/sinatra#1379 Signed-off-by: Robb Kidd <rkidd@chef.io>

* update padrino dependencies (see rack-protection vuln)[sinatra/sinatra#1379] * update actionview dependencies (action-html-sanitizer and loofah)

Name: rack-protection Version: 2.0.0 Advisory: CVE-2018-7212 Criticality: Unknown URL: sinatra/sinatra#1379 Title: Path traversal is possible via backslash characters on Windows.

Because in the previous version of the gemspec we had an open reference to Sinatra it meant we were essentially saying any version would do. Hakiri was flagging this with [CVE-2018-7212](sinatra/sinatra#1379), the resolution of which was to specify a version equal to or greater than 2.0.1

Because in the previous version of the gemspec we had an open reference to Sinatra it meant we were essentially saying any version would do. Hakiri was flagging this with [CVE-2018-7212](sinatra/sinatra#1379), the resolution of which was to specify a version equal to or greater than 2.0.1 It was then flagging this project with [CVE-2018-11627](sinatra/sinatra#1428), and again the resolution was to specify a version, this time equal to or greater than 2.0.2

enhanced path validation in Windows

d17aa95

kgrz reviewed Jan 13, 2018

View reviewed changes

zzak self-assigned this Jan 14, 2018

zzak added bug release vulnerability rack-protection labels Jan 14, 2018

namusyaka added this to the v2.0.1 milestone Feb 6, 2018

namusyaka self-assigned this Feb 9, 2018

Removed the double check line!

ba7af51

Removed the double check line!

namusyaka approved these changes Feb 10, 2018

View reviewed changes

namusyaka merged commit 6bcc6c3 into sinatra:master Feb 11, 2018

namusyaka added a commit to sinatra/rack-protection that referenced this pull request Feb 19, 2018

enhanced path validation in Windows

4239c2f

This commit has been backported from sinatra/sinatra#1379 Fixes CVE-2018-7212

namusyaka mentioned this pull request Feb 19, 2018

enhanced path validation in Windows sinatra/rack-protection#120

Merged

nick-f mentioned this pull request Feb 28, 2018

Update rack-protection to version > 1.5.3 sidekiq/sidekiq#3780

Closed

bess added a commit to curationexperts/epigaea that referenced this pull request Mar 5, 2018

Security updates

51f26e1

nokogiri 1.8.2 fixes security issue https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15412 rack-protection 2.0.1 fixes security issue sinatra/sinatra#1379

bess mentioned this pull request Mar 5, 2018

Security updates curationexperts/epigaea#906

Merged

mkorcy pushed a commit to TuftsUniversity/mira_on_hyrax that referenced this pull request Mar 6, 2018

Security updates

e07b0fd

nokogiri 1.8.2 fixes security issue https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15412 rack-protection 2.0.1 fixes security issue sinatra/sinatra#1379

robbkidd mentioned this pull request Mar 13, 2018

update rack-protection (addresses underlying CVE audit that does not apply to Supermarket installs) chef/supermarket#1728

Merged

kynetiv added a commit to RTICWDT/open-data-maker that referenced this pull request Mar 26, 2018

Update dependencies (#46)

eb2f539

* update padrino dependencies (see rack-protection vuln)[sinatra/sinatra#1379] * update actionview dependencies (action-html-sanitizer and loofah)

shopify-services mentioned this pull request Mar 30, 2018

Path traversal is possible via backslash characters on Windows. Shopify/hosted-payment-sim#46

Closed

rimenes mentioned this pull request May 4, 2018

Update rack-protection gem for security fix. hugopl/reviewit#127

Merged

arkadiybutermanov mentioned this pull request Jun 11, 2018

Safer fix for CVE-2018-7212 ejschmitt/delayed_job_web#107

Merged

johnsyweb mentioned this pull request Nov 15, 2018

Automated Bundle Update envato/event_sourcery_todo_app#25

Merged

Cruikshanks mentioned this pull request Sep 9, 2019

Resolve Hakiri warning for Sinatra version DEFRA/quke-demo-app#15

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enhanced path validation in Windows #1379

enhanced path validation in Windows #1379

orangetw commented Jan 9, 2018

kgrz Jan 13, 2018

orangetw Jan 13, 2018

olleolleolle Jan 15, 2018

kgrz Jan 15, 2018

jkowens Feb 8, 2018

namusyaka Feb 10, 2018

orangetw Feb 10, 2018

namusyaka Feb 10, 2018

orangetw Feb 10, 2018

namusyaka Feb 10, 2018

zzak commented Jan 14, 2018

namusyaka commented Feb 10, 2018

namusyaka left a comment

namusyaka commented Feb 11, 2018

peterkeen commented Feb 27, 2018 •

edited

Loading

andrew-stripe commented Feb 27, 2018

andrew-stripe commented Feb 27, 2018

ghiculescu commented Feb 27, 2018

namusyaka commented Feb 28, 2018 •

edited

Loading

enhanced path validation in Windows #1379

enhanced path validation in Windows #1379

Conversation

orangetw commented Jan 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzak commented Jan 14, 2018

namusyaka commented Feb 10, 2018

namusyaka left a comment

Choose a reason for hiding this comment

namusyaka commented Feb 11, 2018

peterkeen commented Feb 27, 2018 • edited Loading

andrew-stripe commented Feb 27, 2018

andrew-stripe commented Feb 27, 2018

ghiculescu commented Feb 27, 2018

namusyaka commented Feb 28, 2018 • edited Loading

peterkeen commented Feb 27, 2018 •

edited

Loading

namusyaka commented Feb 28, 2018 •

edited

Loading