Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade rubyzip to 1.3.0 for known vulnerability #153

Merged
merged 3 commits into from Oct 7, 2019
Merged

Upgrade rubyzip to 1.3.0 for known vulnerability #153

merged 3 commits into from Oct 7, 2019

Conversation

rhymes
Copy link
Contributor

@rhymes rhymes commented Sep 30, 2019

This updates the minimum required version of rubyzip due to a known vulnerability
rubyzip/rubyzip#403

Due to the fact that this gem downloads zip from known and trusted sources it's probably not an urgent update.

@rhymes
Copy link
Contributor Author

rhymes commented Sep 30, 2019

BTW rubyzip has also released 2.0 with the flag enabled, let me know if you prefer a more elaborate PR that takes that into account as well

@rwojnarowski
Copy link

Selenium relaxed rubyzip version to allow 2.0 ('>= 1.2.2'), https://github.com/SeleniumHQ/selenium/pull/7613/files. I think it's a good idea to do same thing here as well

@kapoorlakshya
Copy link
Collaborator

@rhymes Thanks for the PR! I agree with @rwojnarowski. Can you please change the version requirement to be '>= 1.2.2'?

@woodhull
Copy link

woodhull commented Oct 2, 2019

Yes, it seems important to merge this to allow downstream projects to start using rubyzip 2.0

@rhymes
Copy link
Contributor Author

rhymes commented Oct 3, 2019

@kapoorlakshya I relaxed the version but then I'm thinking we might need to check which version is running for the flag here https://github.com/titusfortner/webdrivers/pull/153/files#diff-148937c344e3c9bb067f1360bd9d14b2R7

because if it's below 1.3.0 that flag doesn't exist

@atsheehan
Copy link

@rhymes do we need to set the global flag on Zip? It might be confusing if an app wants to keep it at false and this gem overrides it.

Maybe a note in the README recommending users upgrade to rubyzip 2.0, or adding the option to the extract method call?

@rhymes
Copy link
Contributor Author

rhymes commented Oct 3, 2019

@atsheehan according to the PR rubyzip/rubyzip#403 - it needs to be set if one uses a version rubyzip >= 1.3.0 and < 2.0.0, if you use a version earlier than 1.3.0 you have the vulnerability, if you use 2.0 the default of that global value is already true.

I'm not sure why we want to allow versions under 1.3.0 TBH but I'm okay either way.

I think we should check the version at runtime and act accordingly in relation to the global flag

@twalpole
Copy link
Collaborator

twalpole commented Oct 4, 2019

I agree there’s no reason to allow < 1.3 on a new release of webdrivers

@anilreddy
Copy link

@kapoorlakshya can you please merge it and make release as we are facing this issue while using webdrivers.

Error:

Unable to activate webdrivers-4.1.2, because rubyzip-2.0.0 conflicts with rubyzip (~> 1.0) (Gem::ConflictError)
C:/Ruby26-x64/lib/ruby/2.6.0/rubygems/specification.rb:2302:in `raise_if_conflicts'
C:/Ruby26-x64/lib/ruby/2.6.0/rubygems/specification.rb:1418:in `activate'
C:/Ruby26-x64/lib/ruby/2.6.0/rubygems.rb:223:in `rescue in try_activate'
C:/Ruby26-x64/lib/ruby/2.6.0/rubygems.rb:216:in `try_activate'
C:/Ruby26-x64/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:123:in `rescue in require'                             C:/Ruby26-x64/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:34:in `require'
F:/Watir/features/support/env.rb:9:in `<top (required)>'
C:/Ruby26-x64/lib/ruby/gems/2.6.0/gems/cucumber-3.1.2/lib/cucumber/glue/registry_and_more.rb:107:in `load'
C:/Ruby26-x64/lib/ruby/gems/2.6.0/gems/cucumber-3.1.2/lib/cucumber/glue/registry_and_more.rb:107:in `load_code_file'
C:/Ruby26-x64/lib/ruby/gems/2.6.0/gems/cucumber-3.1.2/lib/cucumber/runtime/support_code.rb:144:in `load_file'
C:/Ruby26-x64/lib/ruby/gems/2.6.0/gems/cucumber-3.1.2/lib/cucumber/runtime/support_code.rb:85:in `block in load_files!'
C:/Ruby26-x64/lib/ruby/gems/2.6.0/gems/cucumber-3.1.2/lib/cucumber/runtime/support_code.rb:84:in `each'
C:/Ruby26-x64/lib/ruby/gems/2.6.0/gems/cucumber-3.1.2/lib/cucumber/runtime/support_code.rb:84:in `load_files!'
C:/Ruby26-x64/lib/ruby/gems/2.6.0/gems/cucumber-3.1.2/lib/cucumber/runtime.rb:272:in `load_step_definitions'
C:/Ruby26-x64/lib/ruby/gems/2.6.0/gems/cucumber-3.1.2/lib/cucumber/runtime.rb:68:in `run!'
C:/Ruby26-x64/lib/ruby/gems/2.6.0/gems/cucumber-3.1.2/lib/cucumber/cli/main.rb:34:in `execute!'                         C:/Ruby26-x64/lib/ruby/gems/2.6.0/gems/cucumber-3.1.2/bin/cucumber:9:in `<top (required)>'
C:/Ruby26-x64/bin/cucumber:23:in `load'
C:/Ruby26-x64/bin/cucumber:23:in `<main>'

@kapoorlakshya
Copy link
Collaborator

kapoorlakshya commented Oct 7, 2019

@rhymes Let's go ahead and change it to back >= 1.3.0 to make sure a vulnerable version is never used. Thank you!

Edit: I just realized you allowed us to edit the PR so I have made the change myself.

@anilreddy Yes, will release as soon as the requested change is made.

@kapoorlakshya kapoorlakshya merged commit c9cfef0 into titusfortner:master Oct 7, 2019
@kapoorlakshya
Copy link
Collaborator

Released v4.1.3. Let me know if anything breaks.

@rhymes rhymes deleted the rhymes/update-rubyzip-vulnerability branch October 8, 2019 10:04
@rhymes
Copy link
Contributor Author

rhymes commented Oct 8, 2019

@kapoorlakshya thanks for taking it to the finish line!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants