Мы решили в меру своих сил регулярно искать и устранять потенциальные уязвимости и баги в различных проектах. Можно назвать это помощью open-source проектам. Можно - разновидностью рекламы или тестированием анализатора. Еще вариант - очередной способ привлечения внимания к вопросам качества и надёжности кода. На самом деле, не важно название, просто нам нравится это делать. Назовём это необычным хобби. Давайте посмотрим, что интересного было обнаружено в коде различных проектов на этой неделе. Мы нашли время сделать исправления и предлагаем вам ознакомиться с ними.
PVS-Studio — это инструмент, который выявляет в коде многие разновидности ошибок и уязвимостей. PVS-Studio выполняет статический анализ кода и рекомендует программисту обратить внимание на участки программы, в которых с большой вероятностью содержатся ошибки. Наилучший эффект достигается тогда, когда статический анализ выполняется регулярно. Идеологически предупреждения анализатора подобны предупреждениям компилятора. Но в отличии от компиляторов, PVS-Studio выполняет более глубокий и разносторонний анализ кода. Это позволяет ему находить ошибки в том числе и в компиляторах: GCC; LLVM 1, 2, 3; Roslyn.
Поддерживается анализ кода на языках C, C++ и C#. Анализатор работает под управлением Windows и Linux. В Windows анализатор может интегрироваться как плагин в Visual Studio.
Для дальнейшего знакомства с анализатором, предлагаем изучить следующие материалы:
В этом разделе приведены дефекты, которые попадают под классификацию 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 и попробовать проверить ваш проект:
Для снятия ограничения демонстрационной версии, вы можете написать нам, и мы отправим вам временный ключ.
Для быстрого знакомства с анализатором, вы можете воспользоваться утилитами, отслеживающими запуски компилятора и собирающие для проверки всю необходимую информацию. См. описание утилиты CLMonitoring и pvs-studio-analyzer. Если вы работаете с классическим типом проекта в Visual Studio, то всё ещё проще: достаточно выбрать в меню PVS-Studio команду "Check Solution".