Documentation: Don't symbolize tainted data. #14016

Merged
merged 1 commit into from Feb 11, 2014

Conversation

Projects
None yet
5 participants
Contributor

devlinzed commented Feb 11, 2014

I18n.locale= symbolizes its argument, so passing it params[:locale] allows one to DOS your application by visiting ...?locale= URLS repeatedly, with unique values, until the never-GCed symbols monopolize the available memory.

This commit updates the example to include a basic whitelist, but doesn't describe its necessity.

Don't symbolize tainted data.
`I18n.locale=` symbolizes its argument, so passing it `params[:locale]`
allows one to DOS your application by visiting `...?locale=` URLS
repeatedly, with unique values, until the never-GCed symbols monopolize
the available memory.

fxn added a commit that referenced this pull request Feb 11, 2014

Merge pull request #14016 from devlinzed/i18n_doc_fix
Documentation: Don't symbolize tainted data. [ci skip]

@fxn fxn merged commit 4359bc6 into rails:master Feb 11, 2014

1 check failed

default The Travis CI build could not complete due to an error
Details
Owner

fxn commented Feb 11, 2014

A side note of some sort would be nice.

Owner

rafaelfranca commented Feb 11, 2014

You are looking the wrong repository. At the newest version of i18n the whitelist is already there. So it is not calling to_sym in tainted data.

Owner

fxn commented Feb 11, 2014

Oh yes you're right. And i18n did a release for several branches, so this snippet is no longer a good practice, I believe the patch should be reverted.

Owner

rafaelfranca commented Feb 11, 2014

👍

This is a security issue. Are we going to see a rails update in the next days? Is there a CVE?

This doc change has been reverted as I18n already takes care of validating the locales, and was already released with latest Rails versions. Just make sure you have an updated I18n gem.

we are talking about CVE-2013-4492, right?

Owner

rafaelfranca replied Feb 17, 2014

No. This CVE is about something else. I think this i18n change doesn't have any CVE assigned.

do you know which commits are the fixes in i18n?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment