Мы решили в меру своих сил регулярно искать и устранять потенциальные уязвимости и баги в различных проектах. Можно назвать это помощью open-source проектам. Можно — разновидностью рекламы или тестированием анализатора. Еще вариант — очередной способ привлечения внимания к вопросам качества и надёжности кода. На самом деле, не важно название, просто нам нравится это делать. Назовём это необычным хобби. Давайте посмотрим, что интересного было обнаружено в коде различных проектов на этой неделе. Мы нашли время сделать исправления и предлагаем вам ознакомиться с ними.
Для тех, кто ещё не знаком с инструментом PVS-Studio
PVS-Studio — это инструмент, который выявляет в коде многие разновидности ошибок и уязвимостей. PVS-Studio выполняет статический анализ кода и рекомендует программисту обратить внимание на участки программы, в которых с большой вероятностью содержатся ошибки. Наилучший эффект достигается тогда, когда статический анализ выполняется регулярно. Идеологически предупреждения анализатора подобны предупреждениям компилятора. Но в отличии от компиляторов, PVS-Studio выполняет более глубокий и разносторонний анализ кода. Это позволяет ему находить ошибки в том числе и в компиляторах: GCC; LLVM 1, 2, 3; Roslyn.
Поддерживается анализ кода на языках C, C++ и C#. Анализатор работает под управлением Windows и Linux. В Windows анализатор может интегрироваться как плагин в Visual Studio.
Для дальнейшего знакомства с анализатором, предлагаем изучить следующие материалы:
- Подробная презентация на сайте SlideShare. В формате видео она доступна на YouTube (47 минут).
- Статьи о проверенных открытых проектах.
- PVS-Studio: поиск дефектов безопасности.
Потенциальные уязвимости (weaknesses)
В этом разделе приведены дефекты, которые попадают под классификацию CWE и, по сути, являются потенциальными уязвимостями. Конечно, не в каждом проекте дефекты безопасности создают какую-то практическую угрозу, но хочется продемонстрировать, что мы умеем находить подобные ситуации.
1. MSBuild. CWE-476 (NULL Pointer Dereference)
- V3095 The ‘searchLocation’ object was used before it was verified against null. Check lines: 170, 178. Microsoft.Build.Tasks Resolver.cs 170
- V3095 The ‘searchLocation’ object was used before it was verified against null. Check lines: 249, 264. Microsoft.Build.Tasks Resolver.cs 249
- V3095 The ‘assemblyName’ object was used before it was verified against null. Check lines: 176, 194. Microsoft.Build.Tasks Resolver.cs 176
protected bool FileMatchesAssemblyName ( AssemblyNameExtension assemblyName, .... ResolutionSearchLocation searchLocation ) { searchLocation.FileNameAttempted = // <= pathToCandidateAssembly; .... if (String.Compare(assemblyName.Name, ....) != 0) // <= { .... } .... if (searchLocation != null) { .... } .... bool isSimpleAssemblyName = assemblyName == null ? false : assemblyName.IsSimpleName; .... searchLocation.Reason = // <= NoMatchReason.ProcessorArchitectureDoesNotMatch; .... if (searchLocation != null) { .... } .... }Report: https://github.com/Microsoft/msbuild/pull/1891
2. MSBuild. CWE-476 (NULL Pointer Dereference)
V3095 The ‘e’ object was used before it was verified against null. Check lines: 165, 170. MSBuild InitializationException.cs 165
internal static void Throw(string messageResourceName, string invalidSwitch, Exception e, bool showStackTrace) { .... if (showStackTrace) { errorMessage += Environment.NewLine + e.ToString(); // <= } else { errorMessage = ResourceUtilities.FormatString(errorMessage, ((e == null) ? String.Empty : e.Message)); } .... }Report: https://github.com/Microsoft/msbuild/pull/1891
3. Entity Framework. CWE-670 (Always-Incorrect Control Flow Implementation)
V3014 It is likely that a wrong variable is being incremented inside the ‘for’ operator. Consider reviewing ‘i’. EFCore ExpressionEqualityComparer.cs 214
V3015 It is likely that a wrong variable is being compared inside the ‘for’ operator. Consider reviewing ‘i’ EFCore ExpressionEqualityComparer.cs 214
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; .... } }Report: https://github.com/aspnet/EntityFramework/pull/7909
4. Entity Framework. CWE-670 (Always-Incorrect Control Flow Implementation)
V3081 The ‘j’ counter is not used inside a nested loop. Consider inspecting usage of ‘i’ counter. EFCore.Specification.Tests ComplexNavigationsQueryTestBase.cs 2393
for (var i = 0; i < result.Count; i++) { var expectedElement = expected .Single(e => e.Name == result[i].Name); var expectedInnerNames = expectedElement .OneToMany_Optional.Select(e => e.Name) .ToList(); for (var j = 0; j < expectedInnerNames.Count; j++) // <= { Assert.True ( result[i] .OneToMany_Optional.Select(e => e.Name) .Contains(expectedInnerNames[i]) // <= ); } }Report: https://github.com/aspnet/EntityFramework/pull/7909
5. CoreCLR. CWE-188 (Reliance on Data/Memory Layout)
V557 Array overrun is possible. The value of ‘dwCode — 1’ index could reach 8. cordbdi rsmain.cpp 67
const char * GetDebugCodeName(DWORD dwCode) { if (dwCode < 1 || dwCode > 9) { return "!Invalid Debug Event Code!"; } static const char * const szNames[] = { "(1) EXCEPTION_DEBUG_EVENT", "(2) CREATE_THREAD_DEBUG_EVENT", .... "(8) OUTPUT_DEBUG_STRING_EVENT" // <= "(9) RIP_EVENT", }; return szNames[dwCode - 1]; }Report: https://github.com/dotnet/coreclr/pull/10417
6. FreeBSD. CWE-561 (Unreachable code detected)
V779 Unreachable code detected. It is possible that an error is present. mps.c 1306
static int mps_alloc_requests(struct mps_softc *sc) { .... else { panic("failed to allocate command %d\n", i); sc->num_reqs = i; break; } .... }Report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218002
7. FreeBSD. CWE-561 (Unreachable code detected)
V779 Unreachable code detected. It is possible that an error is present. efx_mcdi.c 910
void efx_mcdi_ev_death( __in efx_nic_t *enp, __in int rc) { .... efx_mcdi_raise_exception(enp, emrp, rc); if (emrp != NULL && ev_cpl) emtp->emt_ev_cpl(emtp->emt_context); }Report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218004
8. FreeBSD. CWE-561 (Unreachable code detected)
V779 Unreachable code detected. It is possible that an error is present. sctp_pcb.c 183
struct sctp_vrf * sctp_allocate_vrf(int vrf_id) { .... if (vrf->vrf_addr_hash == NULL) { /* No memory */ #ifdef INVARIANTS panic("No memory for VRF:%d", vrf_id); #endif SCTP_FREE(vrf, SCTP_M_VRF); return (NULL); } .... }Report: bugs.freebsd.org/bugzilla/show_bug.cgi?id=218005
10. FreeBSD. CWE-570 (Expression is Always False)
V547 Expression ‘value < 0’ is always false. Unsigned type value is never < 0. ar9300_xmit.c 450
HAL_BOOL ar9300_reset_tx_queue(struct ath_hal *ah, u_int q) { u_int32_t cw_min, chan_cw_min, value; .... value = (ahp->ah_beaconInterval * 50 / 100) - ah->ah_config.ah_additional_swba_backoff - ah->ah_config.ah_sw_beacon_response_time + ah->ah_config.ah_dma_beacon_response_time; if (value < 10) value = 10; if (value < 0) value = 10; .... }Report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218007
11. FreeBSD. CWE-571 (Expression is Always True)
V617 Consider inspecting the condition. The ‘0x00000080’ argument of the ‘|’ bitwise operation contains a non-zero value. mac_bsdextended.c 128
#define MBO_TYPE_DEFINED 0x00000080 static int ugidfw_rule_valid(struct mac_bsdextended_rule *rule) { .... if ((rule->mbr_object.mbo_neg | MBO_TYPE_DEFINED) && // <= (rule->mbr_object.mbo_type | MBO_ALL_TYPE) != MBO_ALL_TYPE) return (EINVAL); if ((rule->mbr_mode | MBI_ALLPERM) != MBI_ALLPERM) return (EINVAL); return (0); }Report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218039
Прочие ошибки
1. FreeBSD
V646 Consider inspecting the application’s logic. It’s possible that ‘else’ keyword is missing. if_em.c 1944
static int em_if_msix_intr_assign(if_ctx_t ctx, int msix) { .... if (adapter->hw.mac.type < igb_mac_min) { tx_que->eims = 1 << (22 + i); adapter->ims |= tx_que->eims; adapter->ivars |= (8 | tx_que->msix) << (8 + (i * 4)); } if (adapter->hw.mac.type == e1000_82575) // <= tx_que->eims = E1000_EICR_TX_QUEUE0 << (i % adapter->tx_num_queues); else tx_que->eims = 1 << (i % adapter->tx_num_queues); .... }Report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218041
2. CoreCLR
V534 It is likely that a wrong variable is being compared inside the ‘for’ operator. Consider reviewing ‘i’. ildasm mdinfo.cpp 1421
void MDInfo::DisplayFields(mdTypeDef inTypeDef, COR_FIELD_OFFSET *rFieldOffset, ULONG cFieldOffset) { .... for (ULONG i = 0; i < count; i++, totalCount++) { .... for (ULONG iLayout = 0; i < cFieldOffset; ++iLayout) // <= { if (RidFromToken(rFieldOffset[iLayout].ridOfField) == RidFromToken(fields[i])) { .... } } } .... }Report: https://github.com/dotnet/coreclr/pull/10414
Заключение
Предлагаем скачать анализатор PVS-Studio и попробовать проверить ваш проект:
- Скачать PVS-Studio для Windows
- Скачать PVS-Studio для Linux
Для снятия ограничения демонстрационной версии, вы можете написать нам, и мы отправим вам временный ключ.
Для быстрого знакомства с анализатором, вы можете воспользоваться утилитами, отслеживающими запуски компилятора и собирающие для проверки всю необходимую информацию. См. описание утилиты CLMonitoring и pvs-studio-analyzer. Если вы работаете с классическим типом проекта в Visual Studio, то всё ещё проще: достаточно выбрать в меню PVS-Studio команду «Check Solution».
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Weaknesses detected by PVS-Studio this week: episode N3
ссылка на оригинал статьи https://habrahabr.ru/post/324802/
Добавить комментарий