# Интересная ошибка в Entity Framework

В последнее время в качестве хобби и заодно с целью популяризации нашего статического анализатора PVS\-Studio мы выполняем проверки open\-source проектов и по возможности выпускаем патчи с исправлениями\. Сегодня я хотел бы поговорить об одной интересной ошибке, которую я нашёл в проекте Entity Framework\.

Патч, исправляющий данную ошибку, я уже отправил разработчикам, и с ним вы можете ознакомиться по [ссылке](https://github.com/dotnet/efcore/pull/7909)\.

А теперь ближе к делу\. Анализатор выдал 2 предупреждения на одной строке:

* [V3014](https://pvs-studio.ru/ru/docs/warnings/v3014/) It is likely that a wrong variable is being incremented inside the 'for' operator\. Consider reviewing 'i'\. EFCore ExpressionEqualityComparer\.cs 214
* [V3015](https://pvs-studio.ru/ru/docs/warnings/v3015/) It is likely that a wrong variable is being compared inside the 'for' operator\. Consider reviewing 'i' EFCore ExpressionEqualityComparer\.cs 214

Это, кстати, не редкая ситуация, когда анализатор выдаёт 2, а иногда даже 3 предупреждения на одну ошибку\. Все дело в том, что некорректный код может быть аномален сразу с нескольких точек зрения\.

Рассмотрим код:

```cpp
var memberInitExpression = (MemberInitExpression)obj;
....
for (var i = 0; i < memberInitExpression.Bindings.Count; i++)
{
  var memberBinding = memberInitExpression.Bindings[i];
  .... 
  switch (memberBinding.BindingType)
  {
    case ....
    case MemberBindingType.ListBinding:
      var memberListBinding = (MemberListBinding)memberBinding;
      for(var j=0; i < memberListBinding.Initializers.Count; i++)
      {
        hashCode += (hashCode * 397) ^
          GetHashCode(memberListBinding.Initializers[j].Arguments);
      }
      break;
    ....
   }
}
```

Что же здесь происходит? Как мы видим, у нас есть 2 цикла\. В первом используется счетчик с именем _i_ для обхода списка _memberInitExpression\.Bindings_, во втором \- с именем _j_ для обхода списка _memberListBinding\.Initializers_\. Но по какой\-то причине во втором цикле используется счетчик из первого цикла\. Мне это место сразу показалось очень подозрительным, поэтому было решено написать небольшой юнит\-тест, чтобы проверить, действительно ли это ошибка, или хитрый алгоритм программы\.

Код юнит теста:

```cpp
[ConditionalFact]
public void Compare_member_init_expressions_by_hash_code()
{
    MethodInfo addMethod = typeof(List<string>).GetMethod("Add");

    MemberListBinding bindingMessages = Expression.ListBind(
        typeof(Node).GetProperty("Messages"),
        Expression.ElementInit(addMethod, Expression.Constant(
          "Greeting from PVS-Studio developers!"))
    );

    MemberListBinding bindingDescriptions = Expression.ListBind(
        typeof(Node).GetProperty("Descriptions"),
        Expression.ElementInit(addMethod, Expression.Constant(
          "PVS-Studio is a static code analyzer for C, C++ and C#."))
    );

    Expression query1 = Expression.MemberInit(
        Expression.New(typeof(Node)),
        new List<MemberBinding>() {
            bindingMessages                    // One member
        }
    );

    Expression query2 = Expression.MemberInit(
        Expression.New(typeof(Node)),
        new List<MemberBinding>() {
            bindingMessages,                  // Two members
            bindingDescriptions
        }
    );

    var comparer = new ExpressionEqualityComparer();
    var key1Hash = comparer.GetHashCode(query1);
    var key2Hash = comparer.GetHashCode(query2);

    // The hash codes for both expressions 
    // were the same before my edit
    Assert.NotEqual(key1Hash, key2Hash);      // <=
}
```

Мои ожидания подтвердились\. Это настоящая ошибка\. Дело в том, что при сравнении 2 выражений \(Expression's\) всегда сравнивались только первые элементы 2 коллекций, что приводило к некорректному результату для разных выражений с одинаковыми первыми элементами\. Учитывая тот факт, что Entity Framework очень тесно работает с выражениями, и практически основная его цель \- это преобразование лямбд и Linq запросов в SQL запросы, думаю, несложно догадаться, к чему могла приводить столь серьезная ошибка в коде\.

Согласно Common Weakness Enumeration найденную ошибку можно классифицировать как CWE\-670 \(Always\-Incorrect Control Flow Implementation\)\. Неизвестно, можно ли эксплуатировать данную слабость кода как уязвимость, но баг достаточно серьезный\. Это хорошая демонстрация, что анализатор PVS\-Studio можно использовать для поиска потенциальных уязвимостей\. На самом деле он всегда это мог, просто мы не акцентировали на этом внимание\. Подробнее эта тема раскрыта в статье "[PVS\-Studio: поиск дефектов безопасности](https://pvs-studio.ru/ru/blog/posts/cpp/0486/)"\.