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

limit fetching file size & disable XML entity expansion #43

Merged
merged 2 commits into from Oct 23, 2012

Conversation

nov
Copy link
Member

@nov nov commented Oct 22, 2012

Avoid DoS attack to RPs using large XRDS / too many XML entity expansion in XRDS.

@nov
Copy link
Member Author

nov commented Oct 22, 2012

Hi, ruby-openid comitters.

I'm a member of OpenID Foundation.
Yesterday, one PHP guy reported a security issue to OpenID Foundation security ML.
I got forwarded the message from foundation chair and made a quick fix in ruby-openid.
(this security hole seems exist in other languages too)

Basically, this security hole enable DoS attack to any RPs using ruby-openid gem using 100MB+ XRDS file and/or 100,000+ XML external entities.
If you need more info or proof of concept code, please send an email to nov@matake.jp

Thanks

@dennisreimann
Copy link
Contributor

Thanks a lot for bringing this up and providing a fix for it. The tests seem to fail, can you fix them before I'll merge it?

@ubercomp
Copy link

Hi, I'm the one who originally found the bug...

Just a quick correction for posterity: I'm not a PHP guy (don't get me wrong, I love finding bugs on PHP code, especially security flaws). I just happened to come across this flaw for the first time when I was reviewing Drupal [1], and sent nov a proof of concept similar to the one I had included in my original report to the Drupal security team.

Anyway, Nov has all the info, but if you need anything else, feel free to contact me: reginaldo@ubercomp.com

One more thing: any thoughts on updating the sample rails app? I had to install an ancient (v 1.2) rails to get it running...

[1] http://drupal.org/node/1815912

It's not available before ruby 1.9.1 p378.
Use OpenID::HTTPResponse instead.
@nov
Copy link
Member Author

nov commented Oct 23, 2012

OK, now test passes in ruby 1.8 too.

@dennisreimann
Copy link
Contributor

Thank you, Nov!

dennisreimann added a commit that referenced this pull request Oct 23, 2012
limit fetching file size & disable XML entity expansion
@dennisreimann dennisreimann merged commit a3693ce into openid:master Oct 23, 2012
@dennisreimann
Copy link
Contributor

I just released v2.2.2 with this fix. Also big thanks to you @ubercomp for bringing this up! :)

@nov
Copy link
Member Author

nov commented Oct 23, 2012

👍

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

3 participants