We decided to search and fix potential vulnerabilities in various projects. You can call this as you wish - some kind of help to open source projects; a method of promotion or testing of the analyzer. Another way to see it as a way to attract attention to the reliability and quality of the code. In fact, the way to name these posts does not really matter - we just like doing it. This is our little hobby. So, let us have a look at our findings in the code of various projects this week - we had some time to make fixes and suggest looking at them.
PVS-Studio is a tool that detects a large number of types of vulnerabilities and errors in the code. It performs static analysis and points to code fragments that are likely to contain errors. The best effect is achieved when the static analysis is performed regularly. Ideologically, the analyzer warnings are similar to the compiler warnings. However, unlike compilers, PVS-Studio can perform deeper and more versatile code analysis. This enables it to detect errors, even in compilers: GCC; LLVM 1, 2, 3; Roslyn.
The tool supports the analysis of C, C++ and C#; works under Windows and Linux. The analyzer can be integrated as a Visual Studio plug-in.
We suggest the following materials for further investigation of the tool:
In this section we show those defects that fall under the CWE classification and are potential vulnerabilities in their core. Of course, not all weaknesses are really threatening for a project, but we wanted to show that our tool is able to detect them.
1. MSBuild. CWE-476 (NULL Pointer Dereference)
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: https://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
We suggest downloading PVS-Studio analyzer and trying to check your project:
To remove the restrictions of a demo version, you can contact us and we will provide a temporary license key for you.
For a quick introduction to the analyzer, you can use the tools, tracking the runs of the compiler and collect all the necessary information for the analysis. See the description of the utilities CLMonitoring and pvs-studio-analyzer. If you are working with a classic type of project in Visual Studio, everything is much simpler: you should just choose in PVS-Studio menu a command "Check Solution".