2015-09-15 4 views
0

Недавно я побежал Brakeman gem против моего приложения, и один из его предупреждений был о переадресации линии в моем контроллере:Является ли строка, интерполирующая сообщение redirect_to уведомлением о безопасности?

Confidence: High 
Warning type: Redirect 
Message: Possible unprotected redirect near line xx 

В этой строке в моем контроллере, мое редирект уведомительного сообщение содержит имя объекта закачанного пользователь:

def update 
    parent_klass = params[:parent_type].constantize 
    @entity = parent_klass.find params[:parent_id] 

    authorize! :update, @entity 

    entity_param_key = params[:parent_type].downcase.to_sym 
    @entity.update_attributes params[entity_param_key] 

    cache_path = begin 
    if parent_klass == Clinic 
     clinic_path(@entity) 
    else 
     specialist_path(@entity) 
    end 
    end 
    expire_fragment cache_path 

    redirect_to @entity, :notice => "Successfully updated #{@entity.name}." 
end 

в этом контроллере @entity.name является значением формы, которая определяется пользователем, то есть кто-то в теории может попытаться поставить вредоносный код в это поле. Однако я не уверен, что использование этого параметра для создания уведомления представляет собой угрозу безопасности.

сообщения Флэш-уведомление получить представлен в виде, как таковой (в Haml):

#body.container 
    .row 
    #content.span12 
     #container 
     - flash.each do |type, msg| 
      .alert.alert-success= msg 

     = yield 

Является ли эта модель управления риском для безопасности, и если да, то как я могу предотвратить его угрозу безопасности в то же время сохраняя индивидуальное уведомление?

+0

Зависит от того, что вы делаете с этим флэш-сообщением после перенаправления, не так ли? –

+0

@PhilipHallstrom Я просто добавил код вида.Это довольно стандартная визуализация сообщения с уведомлением с использованием bootstrap 2 с некоторым модифицированным CSS. – Kelseydh

+1

HAML автоматически удаляет результат. Если вы использовали '! =' Или имели некоторый материал типа eval, это может быть проблемой, но, как есть, кажется прекрасным. Попробуйте положить что-то противное в себя и посмотреть, что произойдет. –

ответ

2

Предупреждение не относится к флэш-сообщению. Я не уверен, что формат отчета вы читаете, но на выходе текста по умолчанию вы увидите опасное значение подсвечивается + так:

redirect_to(+params[:parent_type].constantize.find(params[:parent_id])+ ... 

в докладе JSON это говорит

"user_input": "params[:parent_type].constantize.find(params[:parent_id])", 

безопасность перенаправления зависит от того, насколько хорошо метод authorize! проверяет результаты params[:parent_type].constantize в действительной модели и (я полагаю), текущий пользователь может изменить его. Если это действительно приводит к модели, перенаправление безопасно.

Однако возможный открытый переадресация является наименьшим из ваших забот в этом методе.

Эти линии позволяют злоумышленнику вызвать find на произвольном классе с произвольным аргументом:

parent_klass = params[:parent_type].constantize 
@entity = parent_klass.find params[:parent_id] 

Хотя это маловероятно, метод find будет опасно, рассмотрим множество кода, вероятно, включенных в приложение и может ли быть какой-то класс где-то с помощью метода find, который вы действительно не хотите, чтобы злоумышленник вызывал.

Следующий бит выглядит как потенциальное массовое назначение:

entity_param_key = params[:parent_type].downcase.to_sym 
@entity.update_attributes params[entity_param_key] 

Учитывая отсутствие permit вызова здесь, я могу предположить, что это приложение не использует сильные параметры. Надеемся, что это белые ключи, доступные для массового назначения, с использованием attr_accessible.

Также params[:parent_type].downcase.to_sym является потенциальной утечкой памяти из создания символа. Обычно это не очень важно в современных приложениях, но в этом случае преобразование не нужно, так как вы можете получить доступ к params с символами или строками.