jgarber / redcloth

RedCloth is a Ruby library for converting Textile into HTML.
Other
443 stars 113 forks source link

Fix CVE-2023-31606 (ReDOS possible in the sanitize_html function) #75

Closed korny closed 11 months ago

korny commented 1 year ago

A potential fix for #73 (https://github.com/e23e/CVE-2023-31606#readme).

The use of a possessive quantifier effectively prevents backtracking in the second group (which matches the tag):

    text.gsub!( /<(\/*)([A-Za-z]\w*+)([^>]*?)(\s?\/?)>/ ) do |m|

In this case, the \w*+ part (and therefore, the whole group ([A-Za-z]\w*+)) will not backtrack. The rest of this regex is not subject to ReDoS (it's linear). So, we can use this version; it should work in Ruby 1.9 and up.

doconnor-clintel commented 1 year ago

@korny - guessing the maintainer of this gem is MIA.

Would it be worth forking the gem, renaming slightly, publishing a new version? (ScarletCloth?) That way there's a migration pathway for others affected; so they don't have to rely on git branches.

Weirdly, https://github.com/jgarber/redcloth/compare/v4.3.2...sofatutor:redcloth:fix-CVE-2023-31606 has a few extra differences which result in

$ bundle exec rake assets:precompile
rake aborted!
LoadError: cannot load such file -- redcloth_scan
Couldn't load redcloth_scan

Though it's not obvious to me why anything there would make it skip trying to compile: https://github.com/jgarber/redcloth#label-Compiling

heliocola commented 1 year ago

@korny , @doconnor-clintel : do you know if the patch has been published to rubygems?

joshuasiler commented 1 year ago

I'm no longer able to maintain this repo but am happy to turn it over to someone with interest and inclination. Feel free to reach out if you are interested.

heliocola commented 11 months ago

@joshuasiler : đź‘‹

joshuasiler commented 11 months ago

I just checked the repo, and it turns out that while I have read/write access, I do not have admin access to the repo. So I am unable to transfer admin. Best bet would be to contact the original owner or fork the repo. Sorry I can't help further.

jgarber commented 11 months ago

I’m admin and would be happy to give out additional access. It’s been on my to-do list to look at this PR but I haven’t gotten to it, yet.

From: Joshua Siler @.> Date: Friday, October 27, 2023 at 12:55 PM To: jgarber/redcloth @.> Cc: Subscribed @.***> Subject: Re: [jgarber/redcloth] Fix CVE-2023-31606 (ReDOS possible in the sanitize_html function) (PR #75)

I just checked the repo, and it turns out that while I have read/write access, I do not have admin access to the repo. So I am unable to transfer admin. Best bet would be to contact the original owner or fork the repo. Sorry I can't help further.

— Reply to this email directly, view it on GitHubhttps://github.com/jgarber/redcloth/pull/75#issuecomment-1783226613, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAAB67ORN3POVSVYSODM7CTYBPRRBAVCNFSM6AAAAAAZXLBSKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBTGIZDMNRRGM. You are receiving this because you are subscribed to this thread.Message ID: @.***>

digitalmoksha commented 11 months ago

I’m admin and would be happy to give out additional access. It’s been on my to-do list to look at this PR but I haven’t gotten to it, yet.

@jgarber that would be great if that's a possibility.

I work for GitLab (https://gitlab.com/digitalmoksha). We still use RedCloth - it’s not a huge component for us, but we do still use it. I also use it on a couple of older personal projects.

I would be interested in being able to maintain it, at the very least getting the security fixes published.

Do you think this would be a possibility? I don’t have a huge amount of time to devote to it, but I would rather see the security problems dealt with in the main gem rather than relying on forks. And there are a couple other fixes that I think would be useful for the community.

wdyt?

heliocola commented 11 months ago

@jgarber : rooting for you to pick either @digitalmoksha or myself as a new admins! Or somebody else you have in mind that also has interest and time! đź’ź

jgarber commented 11 months ago

@digitalmoksha @heliocola Sent you both access. Thank you so much for stepping up! 🙏 Go to town with fixes and enhancements. I, too, would like people not to have to rely on forks! It's wonderful if RedCloth can still be useful to some people out there. Sorry I can't maintain it myself!

digitalmoksha commented 11 months ago

Thank you @jgarber 🙇

heliocola commented 11 months ago

Thank you @jgarber !

jgarber commented 11 months ago

@heliocola You’re a rubygems admin now. Thank you so much for getting these fixes to a release!