Исправление CWE-ID 100 для MVC5

Согласно Veracode, наше приложение несколько сотен раз обнаруживали "недостатки" CWE-ID 100, связанные с специфическими технологическими проблемами проверки ввода.

Согласно их документам, исправление заключается в проверке свойства ModelState.IsValid в модели перед его использованием. Мы делаем это при каждом действии контроллера, но мы все еще звоним. Ниже приведен пример действия контроллера.

public async Task<ActionResult> DeliverySummary (ReportsViewModel Model)
{
    if (ModelState.IsValid)
    {
        /* Other processing occurs here */ 

        //finally return View
        return View(Model);
    }
    else 
    {
        return View();
    }
}

У нас есть System.ComponentModel.DataAnnotations в свойствах нашей модели.

Кто-нибудь когда-нибудь сталкивался с этим?


c# asp.net-mvc asp.net-mvc-5 veracode
person mituw16    schedule 07.06.2017    source источник
comment
Опубликуйте свойства вашей модели и какие свойства вызвали проблему CWE. Я искал аналогичную проблему на SO и только что нашел здесь одну запись: for" title="veracode создает проблемы с проверкой ввода, специфичные для технологии, cwe id 100 for">stackoverflow.com/questions/44289347/.   -  person Tetsuya Yamamoto    schedule 08.06.2017
comment
Наши модели просмотра огромны.. Хотите увидеть все это? Они не сказали, какие свойства повлияли на результаты сканирования. Вот в этом и вся проблема.. Я не знаю, за что они нам звонят :/   -  person mituw16    schedule 08.06.2017

Ответы (1)


arrow_upward
5
arrow_downward

Я сам занимался этим. Основная проблема заключается в том, что вы не установили [Bind] для своего аргумента, указав разрешенные свойства.

Мое старое действие контроллера входа было таким

public ActionResult SignIn(SignInViewModel viewModel)

И чтобы исправить это, мне нужно, чтобы это читалось так

public ActionResult SignIn([Bind(Include = "Email,Password,UtcOffset")]SignInViewModel viewModel)

Это говорит MVC о том, что только свойства Email, Password и UtcOffset будут считаны из SignInViewModel, поэтому, если хакер также установит LastLogonTime, это будет проигнорировано.

Кроме того, из-за проверок безопасности от Veracode я думаю, что такая привязка модели теперь невероятно неудобна, учитывая, что разработчики теперь должны синхронизировать строки с именами свойств в цели. Какая проблема.

person Todd Sprang    schedule 13.06.2017
comment
Я также наткнулся на Bind возможное решение и реализовал его на нескольких действиях контроллера для тестирования, но недостатки все еще присутствовали в результатах сканирования. Мы открыли заявку в службу технической поддержки с помощью Veracode после того, как один из их специалистов по безопасности не обнаружил проблем с нашим кодом во время консультационного звонка. Он был сбит с толку тем, почему нас звонят. Я отпишусь, что услышу, как только они разберутся с этим - person mituw16; 13.06.2017
comment
Я также полностью согласен с тем, что добавлять Bind к каждому действию контроллера невероятно неуклюже. Почти все исследования, которые я провел в отношении этого недостатка, в частности, похоже, вращаются вокруг структуры Entity и прямых изменений базы данных из вашей модели, которая передается в контроллер. Мы вообще не используем EF и полностью абстрагируем наш DAL, следуя шаблону репозитория с хранимыми процедурами. Я бы хотел, чтобы платформа Veracode была достаточно умной, чтобы понимать, когда EF используется или нет, и соответствующим образом сообщать о недостатках EF. - person mituw16; 13.06.2017
comment
@mituw16 mituw16 вы получили ответ от Veracode по поводу вашего запроса? Мы также сталкиваемся с той же проблемой в нашем приложении. - person hgarg; 13.02.2018
comment
@hgarg На самом деле, да. Им нужно было внести изменения в свой сканирующий механизм. У них была проблема в их механизме сканирования, связанная с нашим использованием действия асинхронного контроллера, которое вызывало проблему. Я наконец заметил, что их сканеры не имели проблем с неасинхронными действиями контроллера. После того, как мы это выяснили, они исправили проблему со своим сканером, и все было хорошо. Честно говоря, весь процесс представлял собой огромную пробежку, которая заняла почти 2 месяца, прежде чем они наконец признали, что в их движке была ошибка. - person mituw16; 13.02.2018