Андрей Карпов

31 Мар 2017

Теги:

#Cpp #Security

Дефекты безопасности, которые устранила команда PVS-Studio на этой неделе: выпуск N4

31 Мар 2017

Автор: Андрей Карпов

Для тех, кто ещё не знаком с инструментом PVS-Studio
Потенциальные уязвимости (weaknesses)
Прочие ошибки
Заключение

Мы решили в меру своих сил регулярно искать и устранять потенциальные уязвимости и баги в различных проектах. Можно назвать это помощью open-source проектам. Можно - разновидностью рекламы или тестированием анализатора. Еще вариант - очередной способ привлечения внимания к вопросам качества и надёжности кода. На самом деле, не важно название, просто нам нравится это делать. Назовём это необычным хобби. Давайте посмотрим, что интересного было обнаружено в коде различных проектов на этой неделе. Мы нашли время сделать исправления и предлагаем вам ознакомиться с ними.

Для тех, кто ещё не знаком с инструментом PVS-Studio

PVS-Studio — это инструмент, который выявляет в коде многие разновидности ошибок и уязвимостей. PVS-Studio выполняет статический анализ кода и рекомендует программисту обратить внимание на участки программы, в которых с большой вероятностью содержатся ошибки. Наилучший эффект достигается тогда, когда статический анализ выполняется регулярно. Идеологически предупреждения анализатора подобны предупреждениям компилятора. Но в отличии от компиляторов, PVS-Studio выполняет более глубокий и разносторонний анализ кода. Это позволяет ему находить ошибки в том числе и в компиляторах: GCC; LLVM 1, 2, 3; Roslyn.

Поддерживается анализ кода на языках C, C++ и C#. Анализатор работает под управлением Windows и Linux. В Windows анализатор может интегрироваться как плагин в Visual Studio.

Для дальнейшего знакомства с анализатором, предлагаем изучить следующие материалы:

В формате видео презентация доступна на YouTube (47 минут).
Статьи о проверенных открытых проектах.
PVS-Studio: поиск дефектов безопасности.

Потенциальные уязвимости (weaknesses)

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

1. CryEngine V. CWE-806 (Buffer Access Using Size of Source Buffer)

V512 A call of the 'memcpy' function will lead to underflow of the buffer 'hashableData'. GeomCacheRenderNode.cpp 285

void CGeomCacheRenderNode::Render(....)
{
  ....
  CREGeomCache* pCREGeomCache = iter->second.m_pRenderElement;
  ....
  uint8 hashableData[] =
  {
   0, 0, 0, 0, 0, 0, 0, 0,
   (uint8)std::distance(pCREGeomCache->....->begin(), &meshData),
   (uint8)std::distance(meshData....->....begin(), &chunk),
   (uint8)std::distance(meshData.m_instances.begin(), &instance)
  };

  memcpy(hashableData,pCREGeomCache,sizeof(pCREGeomCache)); // <=
  ....
}

2. CryEngine V. CWE-467 (Use of sizeof() on a Pointer Type)

V568 It's odd that 'sizeof()' operator evaluates the size of a pointer to a class, but not the size of the 'this' class object. ClipVolumeManager.cpp 145

void
CClipVolumeManager::GetMemoryUsage(class ICrySizer* pSizer) const
{
  pSizer->AddObject(this, sizeof(this));
  for (size_t i = 0; i < m_ClipVolumes.size(); ++i)
    pSizer->AddObject(m_ClipVolumes[i].m_pVolume);
}

3. CryEngine V. CWE-571 (Expression is Always True)

V501 There are identical sub-expressions to the left and to the right of the '==' operator: bActive == bActive LightEntity.h 124

void SetActive(bool bActive)
{
  if (bActive == bActive)
    return;

  m_bActive = bActive;
  OnResetState();
}

4. CryEngine V. CWE-476 (NULL Pointer Dereference)

V595 The 'pTrack' pointer was utilized before it was verified against nullptr. Check lines: 60, 61. AudioNode.cpp 60

void CAudioNode::Animate(SAnimContext& animContext)
{
  ....
  const bool bMuted = gEnv->IsEditor() && (pTrack->GetFlags() &
    IAnimTrack::eAnimTrackFlags_Muted);
  if (!pTrack || pTrack->GetNumKeys() == 0 ||
       pTrack->GetFlags() & IAnimTrack::eAnimTrackFlags_Disabled)
  {
    continue;
  }
  ....
}

5. CryEngine V. CWE-688 (Function Call With Incorrect Variable or Reference as Argument)

V549 The first argument of 'memcpy' function is equal to the second argument. ObjectsTree_Serialize.cpp 1135

void COctreeNode::LoadSingleObject(....)
{
  ....
  float* pAuxDataDst = pObj->GetAuxSerializationDataPtr(....);
  const float* pAuxDataSrc = StepData<float>(....);
  memcpy(pAuxDataDst, pAuxDataDst, min(....) * sizeof(float));
  ....
}

6. LLVM. CWE-476 (NULL Pointer Dereference)

V595 The 'DIExpr' pointer was utilized before it was verified against nullptr. Check lines: 949, 950. codeviewdebug.cpp 949

void CodeViewDebug::collectVariableInfo(const DISubprogram *SP) {
  ....
  const DIExpression *DIExpr = DVInst->getDebugExpression();
  bool IsSubfield = false;
  unsigned StructOffset = 0;

  // Handle fragments.
  auto Fragment = DIExpr->getFragmentInfo();   // <=
  if (DIExpr && Fragment) {                    // <=
    IsSubfield = true;
    StructOffset = Fragment->OffsetInBits / 8;
  } else if (DIExpr && DIExpr->getNumElements() > 0) {
    continue; // Ignore unrecognized exprs.
  }
  ....
}

Bug Report: https://bugs.llvm.org/show_bug.cgi?id=32430

7. LLVM. CWE-476 (NULL Pointer Dereference)

V595 The 'Initializer' pointer was utilized before it was verified against nullptr. Check lines: 335, 338. semaoverload.cpp 335

NarrowingKind
StandardConversionSequence::getNarrowingKind(....) const {
  ....
  const Expr *Initializer = IgnoreNarrowingConversion(Converted);

  if (Initializer->isValueDependent()) // <=
    return NK_Dependent_Narrowing;

  if (Initializer &&                   // <=
      Initializer->isIntegerConstantExpr(IntConstantValue, Ctx)){
  ....
}

Bug Report: https://bugs.llvm.org/show_bug.cgi?id=32447

8. RPCS3. CWE-570 (Expression is Always False)

V547 Expression 'sock < 0' is always false. Unsigned type value is never < 0. sys_net.cpp 695

#ifdef _WIN32
  using socket_t = SOCKET;
#else
  using socket_t = int;
#endif
s32 socket(s32 family, s32 type, s32 protocol)
{
  ....
  socket_t sock = ::socket(family, type, protocol);

  if (sock < 0)
  {
    libnet.error("socket()....", get_errno() = get_last_error());
    return -1;
  }
  ....
}

Pull Request: https://github.com/RPCS3/rpcs3/pull/2543

Прочие ошибки

1. CoreCLR

V778 Two similar code fragments were found. Perhaps, this is a typo and 'IMAGE_LOADED_FOR_INTROSPECTION' variable should be used instead of 'IMAGE_LOADED'. cee_dac peimage.cpp 811

void PEImage::EnumMemoryRegions(CLRDataEnumMemoryFlags flags)
{
  ....
  if (m_pLayouts[IMAGE_LOADED].IsValid() &&
      m_pLayouts[IMAGE_LOADED]!=NULL)
    m_pLayouts[IMAGE_LOADED]->EnumMemoryRegions(flags);

  if (m_pLayouts[IMAGE_LOADED_FOR_INTROSPECTION].IsValid() &&
      m_pLayouts[IMAGE_LOADED]!=NULL)                   // <=
    m_pLayouts[IMAGE_LOADED_FOR_INTROSPECTION]->
      EnumMemoryRegions(flags);
}

Pull Request: https://github.com/dotnet/coreclr/pull/10450

2. CoreCLR

V778 Two similar code fragments were found. Perhaps, this is a typo and 'weight2' variable should be used instead of 'weight1'. clrjit lclvars.cpp 2702

int __cdecl Compiler::RefCntCmp(const void* op1, const void* op2)
{
  ....
  if (weight1)
  {
    ....
    if (varTypeIsGC(dsc1->TypeGet()))
    {
      weight1 += BB_UNITY_WEIGHT / 2;
    }
    if (dsc1->lvRegister)
    {
      weight1 += BB_UNITY_WEIGHT / 2;
    }
  }

  if (weight1)
  {
    ....
    if (varTypeIsGC(dsc2->TypeGet()))
    {
      weight1 += BB_UNITY_WEIGHT / 2;  // <=
    }
    if (dsc2->lvRegister)
    {
      weight2 += BB_UNITY_WEIGHT / 2;
    }
  }
  ....
}

Pull Request: https://github.com/dotnet/coreclr/pull/10450

3. CoreCLR

V778 Two similar code fragments were found. Perhaps, this is a typo and 'g_szBuf_ProperName' variable should be used instead of 'g_szBuf_UnquotedProperName'. ildasm dasm.cpp 486

void Uninit()
{
  ....
  if (g_szBuf_UnquotedProperName != NULL)
  {
    SDELETE(g_szBuf_UnquotedProperName);
  }
  if (g_szBuf_UnquotedProperName != NULL)  // <=
  {
    SDELETE(g_szBuf_ProperName);
  }
  ....
}

Pull Request: https://github.com/dotnet/coreclr/pull/10450

4. LLVM

V778 Two similar code fragments were found. Perhaps, this is a typo and 'FS' variable should be used instead of 'TS'. hexagonearlyifconv.cpp 549

bool HexagonEarlyIfConversion::isProfitable(....) const
{
  ....
  unsigned TS = 0, FS = 0, Spare = 0;
  if (FP.TrueB) {
    TS = std::distance(FP.TrueB->begin(),
      FP.TrueB->getFirstTerminator());
    if (TS < HEXAGON_PACKET_SIZE)
      Spare += HEXAGON_PACKET_SIZE-TS;     // <=
  }
  if (FP.FalseB) {
    FS = std::distance(FP.FalseB->begin(),
      FP.FalseB->getFirstTerminator());
    if (FS < HEXAGON_PACKET_SIZE)
      Spare += HEXAGON_PACKET_SIZE-TS;     // <=
  }
  unsigned TotalIn = TS+FS;
  ....
}

Bug Report: https://bugs.llvm.org/show_bug.cgi?id=32480

Заключение

Предлагаем скачать анализатор PVS-Studio и попробовать проверить ваш проект:

Скачать PVS-Studio для Windows
Скачать PVS-Studio для Linux

Для снятия ограничения демонстрационной версии, вы можете написать нам, и мы отправим вам временный ключ.

Для быстрого знакомства с анализатором, вы можете воспользоваться утилитами, отслеживающими запуски компилятора и собирающие для проверки всю необходимую информацию. См. описание утилиты CLMonitoring и pvs-studio-analyzer. Если вы работаете с классическим типом проекта в Visual Studio, то всё ещё проще: достаточно выбрать в меню PVS-Studio команду "Check Solution".

#Cpp #Security