search

rails / rails

Ruby on Rails
https://rubyonrails.org
MIT License
56.04k stars 21.67k forks source link

Please release a 6.1 version with a patch for CVE-2024-26143 #51190

Closed radar closed 8 months ago

radar commented 8 months ago

This commit: https://github.com/rails/rails/commit/5187a9ef51980ad1b8e81945ebe0462d28f84f9e is present on the 7-1-stable and 7-0-stable branches.

As per Rails Maintenance Policy, 6-1-stable should have also received this patch.

Could you please add this patch to 6-1-stable as well?

canderson-activatecare commented 8 months ago

This should also include #51156 which was corrected a regression in the above mentioned commit.

Edit: For clarification, the PR mentioned above moves some of the code from 5187a9ef51980ad1b8e81945ebe0462d28f84f9e into the proper ActiveSupport lib activesupport/lib/active_support/html_safe_translation.rb.

rubylevelup commented 8 months ago

Some output: 

ruby-advisory-db:
  advisories:   874 advisories
  last updated: 2024-02-24 17:04:04 -0800
  commit:       5d80bde618c0836393581d7c8eb9ca61600a255f
Name: actionpack
Version: 6.1.7.7
CVE: CVE-2024-26143
Criticality: Unknown
URL: https://discuss.rubyonrails.org/t/possible-xss-vulnerability-in-action-controller/84947
Title: Possible XSS Vulnerability in Action Controller
Solution: upgrade to '~> 7.0.8, >= 7.0.8.1', '>= 7.1.3.1'

Vulnerabilities found!
jgarber623 commented 8 months ago

@radar Thanks for opening this issue. I ran head-first into the same problem this morning. I rolled the dice on a fix and opened up #51196.

rafaelfranca commented 8 months ago

Thank you for the issue. I'll update the maintenance policy page to reflect the reality.

6.1 only receives critical security upgrades and this ins't critical.

See https://rubyonrails.org/2024/2/21/Rails-Versions-6-1-7-7-7-0-8-1-and-7-1-3-2-have-been-released

This is a reminder that the 6.1 series only receives critical security updates. Users are encouraged to upgrade as soon as possible.

Fryguy commented 8 months ago

@rafaelfranca Just to clarify - you will be changing the Security Issues section roughly to the following?

- Currently included series: 7.1.Z, 7.0.Z, 6.1.Z.
+ Currently included series: 7.1.Z, 7.0.Z.

and the Severe Security Issues section will stay as is?

rafaelfranca commented 8 months ago

Yes https://github.com/rails/rails/commit/79cde06437c793a87fef3a7a73ec8a7b93ddcf05

radar commented 8 months ago

Arbitrarily changing the contract of security updates because you don’t feel like it… this stinks.

andrewroth commented 8 months ago

The lack of a 6.1 fix is causing significant problems today for me and my organization as well.

rafaelfranca commented 8 months ago

What do you mean by arbitrarily changing? It is literally written in the policy and I didn't change that.

4 Security Issues The current release series and the next most recent one will receive patches and new versions in case of a security issue.

The current release series: 7.1.z and the next most recent one: 7.0.z

jbirdjavi commented 8 months ago

Advance notice before removing a version from the "Currently included series" section is appreciated. I would say that "arbitrarily" is correct in this sense. Removing a version after a CVE is disclosed is not helpful for those of us maintaining lots of Rails apps.

radar commented 8 months ago

Yes, that's the opening para for that section. But then the bottom bit says:

Currently included series: 7.1.Z, 7.0.Z, 6.1.Z.

This change is a cherry-pick, gem push and (optional) blog post away from being on 6-1-stable. You'd get kudos from those of us who are, because the guide said so (and for our own reasons, usually time-pressure ones), still maintaining Rails 6.1 apps.

If we knew that 6.1 wasn't receiving security patches, we would've upgraded sooner. Pulling the rug out here isn't the right move. What would be better is:

  1. Apply this security patch to 6-1-stable, so that bundle audit runs cleanly on updates
  2. Provide advice, as you've already done, on 6-1-stable no longer receiving security patches. Perhaps set a timeline of 3-6 months to give people time to upgrade.

"Surprise, your version of Rails is no longer receiving security updates because we're not feeling the vibe" isn't it.

rafaelfranca commented 8 months ago

The policy is on the opening paragraph, but the included session is on our discretion how long things stay there, but I totally can see how this is confusing.

I also thought I announced 6.1.z would only get severe security releases in the 7.1 release post but I didn't. That is my mistake, and I'm sorry.

Here is what I'm going to do:

  1. Finally implement a maintenance policy that is clear on which versions are maintained and until when
  2. Release the patch for the mentioned issue on 6.1
  3. Properly announce 6.1 is out of maintenance and define a future date for it.
  4. Personally guarantee 6.1 is maintained until that date to not burden the other maintainers with my mistakes

Thank you for explaining the confusion.

rafaelfranca commented 8 months ago

Hey folks, good news!

It seems a patch on 6.1 isn't necessary. The behavior that is vulnerable isn't present in that version. Until 7.0.0, translate on controllers didn't mark any strings as HTML safe, so it can't make applications vulnerable to XSS attacks.

I'll still do the other things that I listed above, along with an ament to the CVE to make clear no version < 7.0.0 is vulnerable.

radar commented 8 months ago

Thank you for understanding and for your work here. :)

rafaelfranca commented 8 months ago

Done https://discuss.rubyonrails.org/t/possible-xss-vulnerability-in-action-controller/84947/3?u=rafaelfranca

Also opened a PR to ruby-advisory-db that I believe is what bundle audit uses https://github.com/rubysec/ruby-advisory-db/pull/753

jgarber623 commented 8 months ago

I'm both glad to see this properly resolved and absolutely regret getting involved.

Thanks for sticking with it, @radar.