search

ejschmitt / delayed_job_web

Resque like web interface for delayed job
MIT License
478 stars 188 forks source link

Safer fix for CVE-2018-7212 #107

Closed arkadiybutermanov closed 6 years ago

arkadiybutermanov commented 6 years ago

https://github.com/ejschmitt/delayed_job_web/pull/103 introduced a major version upgrade for sinatra from v1.4.4 to v2.0.1. Unfortunately, this doesn't allow us to continue using delayed_job_web in old projects, that depend on sinatra 1.x (we have to bump delayed_job_web to eliminate latest CVEs).

The original CVE fix (https://github.com/sinatra/sinatra/pull/1379) was introduced for sinatra 2.x only, but it was also backported to the rack-protection gem, which is used in sinatra 1.x: https://github.com/sinatra/rack-protection/pull/120 Also, see https://github.com/rubysec/ruby-advisory-db/pull/331

So, I think, we should revert changes introduced in https://github.com/ejschmitt/delayed_job_web/pull/103 and instead explicitly require the latest version of rack-protection to address CVE. This will allow us to use the latest (and the safest) version of delayed_job_web in both old and new projects.

BTW, are you guys following https://semver.org? It was pretty surprising to see, that patch (1.4.1) introduced a major dependency upgrade :)

andyatkinson commented 6 years ago

@arkadiybutermanov Thanks for pointing out the upgrade of the dependency on the patch release version, that was a mistake, I agree. Your change makes sense.

andyatkinson commented 6 years ago

@arkadiybutermanov 1.4.3 was just released. Can you give it a shot in your app and report back? Thanks.

arkadiybutermanov commented 6 years ago

@andyatkinson , thanks for super quick feedback! Tried it in my app - I was able to upgrade delayed_job_web to 1.4.3 without any issues 👍

jankeesvw commented 6 years ago

Hi @andyatkinson and @arkadiybutermanov,

Have you seen there is a new security hole in Sinatra? This time it's CVE-2018-11627:

Sinatra before 2.0.2 has XSS via the 400 Bad Request page that occurs upon a params parser exception.

How do you think we should solve this?

arkadiybutermanov commented 6 years ago

@jankeesvw , sinatra 1.x doesn't seem to be vulnerable: https://github.com/sinatra/sinatra/issues/1428#issuecomment-395395403 and https://github.com/sinatra/sinatra/issues/1428#issuecomment-395434408

jankeesvw commented 6 years ago

@jankeesvw , sinatra 1.x doesn't seem to be vulnerable

Good to know!