Bike's brake

Como contornar erros no brakeman: “User controlled method execution” e “Unsafe reflection method constantize called with parameter value”

Eu tenho dois models diferentes mas que ambos respondem aos mesmos métodos, no esquema duck typing, e preciso criar uma action show pra eles.

Porém, se eles respondem aos mesmos métodos, pra que vou criar 2 actions show em 2 controllers diferentes se elas serão exatamente iguais?

Desenvolvedor Ruby malandro que é, você cria uma rota que vai te passar uma referência ao model, além do ID, e me lança a seguinte action:

# routes.rb
get 'clients/:contactable/:id', to: 'clients#show', as: :contactable

# ClientsController
def show
  contactable_model = params[:contactable]
  @contactable = current_company.send(contactable_model).find(params[:id])
  ...
end

E aí vem o nosso querido Brakeman, sempre preocupado com as falhas de segurança da nossa aplicação, lança o erro: “User controlled method execution”!

Você relê o código que escreveu e na mesma hora vê o mole que deu!  Usando send com qualquer coisa que o usuário preencher!?

Como resolver isso? Você pensa em usar o constantize porque se o usuário tentar marretar qualquer outra coisa vai dar exception de constante não existe, easy:

def show
  contactable_model = params[:contactable].singularize.classify.constantize
  @contactable = contactable_model.where(company: current_company).find(params[:id])
end

Aí vem o Brakeman e lança: “Unsafe reflection method constantize called with parameter value”…

Você para um pouco e reflete sobre isso… verdade, ainda é muita informação que você tá dando pro possível atacante… como resolver então?

A forma que mais curti foi a sugerida pelo Paul Kwiatkowski:

class ClientsController < ApplicationController
  CONTACTABLE_MODELS = {
    'contacts' => Contact,
    'connections' => Connection
  }.freeze

  def show
    contactable_model = CONTACTABLE_MODELS.fetch(params[:contactable])
    @contactable = contactable_model.where(company: current_company).find(params[:id])
  end
end

Dessa forma, se o atacante tentar qualquer outro valor além dos que estão nas chaves da constante, uma exception será levantada de cara pelo fetch. 👍

Leave a Comment