Security defects that PVS-Studio team fixed this week: Issue N3

We have decided, as far as possible, to regularly search for and fix potential vulnerabilities and bugs in various projects. You can call this using open-source projects. You can - a kind of advertising or testing analyzer. Another option is another way to draw attention to the issues of code quality and reliability. Actually, the name doesn't matter, we just like to do it. Let's call it an unusual hobby. Let's see what interesting things were found in the code of various projects this week. We took the time to make corrections and invite you to read them.

For those who are not familiar with the PVS-Studio tool yet

PVS-Studio is a tool that reveals many types of errors and vulnerabilities in the code. PVS-Studio performs static code analysis and recommends the programmer to pay attention to areas of the program that are likely to contain errors. The best effect is achieved when static analysis is performed regularly. Ideologically, analyzer warnings are similar to compiler warnings. But unlike compilers, PVS-Studio performs a deeper and more versatile code analysis. This allows him to find errors, including in compilers: GCC ; LLVM 1 , 2 , 3 ; Roslyn .

Code analysis in C, C ++ and C # languages ​​is supported. The analyzer runs on Windows and Linux. In Windows, the analyzer can be integrated as a plugin in Visual Studio.

For further acquaintance with the analyzer, we suggest studying the following materials:
')

• Detailed presentation on SlideShare website. In video format, it is available on YouTube (47 minutes).
• Articles about proven open source projects.
• PVS-Studio: search for security defects .

Potential vulnerabilities (weaknesses)

This section lists defects that fall under the CWE classification and, in fact, are potential vulnerabilities. Of course, not every project has security defects that pose some kind of practical threat, but I want to demonstrate that we can find such situations.

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

2. MSBuild. CWE-476 (NULL Pointer Dereference)

V3095 The 'e' object was used against it. 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

3. Entity Framework. CWE-670 (Always-Incorrect Control Flow Implementation)

V3014 It is likely that a variable is being incremented inside the 'for' operator. Consider reviewing 'i'. EFCore ExpressionEqualityComparer.cs 214

V3015 It is likely that a 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

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

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

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

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

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

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

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

Other errors

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

2. CoreCLR

V534 It is likely that a 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

Conclusion

We offer to download the PVS-Studio analyzer and try to check your project:

• Download PVS-Studio for Windows
• Download PVS-Studio for Linux

To remove the demo version limitation , you can email us and we will send you a temporary key.

For a quick acquaintance with the analyzer, you can use utilities that track compiler launches and collect all the necessary information for verification. See the CLMonitoring and pvs-studio-analyzer utility descriptions. If you are working with a classic project type in Visual Studio, it is still easier: just select the “Check Solution” command in the PVS-Studio menu.



If you want to share this article with an English-speaking audience, then please use the link to the translation: Andrey Karpov. Weaknesses detected by PVS-Studio this week: episode N3