Blacklist "original_script_name" get param

[viseztrance]

I was looking over the source code, and I noticed that the get param black list was incomplete. Passing the original_script_name param will redirect to other domains.

[dee-see]

Hi @amatsuda, given that this has security implications, would you consider a 1.2.1 release that contains this fix? Thanks!

[olliebennett]

The changelog suggests this corresponds to CVE-2020-11082. Is that still to be published?

Are any more details available explaining the potential impact of this?

I see mention of original_script_name in Rails' UrlFor module, but little context or documentation.

Thanks

[hsbt]

@olliebennett see GHSA-r5jw-62xg-j433

[JamesChevalier]

Is there a workaround that will work with version 0.17.0?
It doesn't appear that PARAM_KEY_EXCEPT_LIST is defined in that version of the gem.

https://github.com/kaminari/kaminari/blob/0-17-stable/lib/kaminari/helpers/tags.rb#L1-L15

[utkarsh2102]

Same question as above for 0.16.0^
Is it even affected?

CC: @amatsuda, @yuki24, @hsbt

[yuki24]

For those of you who are still using a version below 1.0.0, please use the following patch to mitigate the security risk:

# config/initializers/kaminari.rb
module KaminariSecurityPatch
  prepend_features Kaminari::Helpers::Tag

  PARAM_KEY_DENIED_LIST = %w[ authenticity_token commit utf8 method script_name original_script_name ].freeze

  def page_url_for(page)
    params = @params
               .merge(@param_name => (page <= 1 ? nil : page), only_path: true)
               .except(*PARAM_KEY_DENIED_LIST)

    @template.url_for(params)
  end
end

Edit: The script has been updated to use string keys as @params has string keys, not symbol keys. Thanks to @JamesChevalier for pointing that out.

[utkarsh2102]

You're awesome, thanks! ❤️

[JamesChevalier]

Seems like @params is a Hash with string keys, so the except(*PARAM_KEY_DENIED_LIST) didn't find/remove :original_script_name because it existed in the hash as "original_script_name"

I think I have two options to adjust your code:

Change the constant:

PARAM_KEY_DENIED_LIST = ["authenticity_token", "commit", "utf8", "_method", "script_name", "original_script_name"].freeze

Or symbolize the keys:

params = @params.merge(@param_name => (page <= 1 ? nil : page), only_path: true).symbolize_keys.except(*PARAM_KEY_DENIED_LIST)

Thanks!

[yuki24]

@JamesChevalier Thanks for pointing that out. You are right that they are in fact string objects. We should use string keys as symbols could exhaust the GC in versions older than 2.2. I have just updated the snippet above.