Permalink
Browse files

Fix issues detected by Secure Development Life check. (SDL-7.0) (#1356)

* Add security development lifecycle checks (SDL-7.0)

* Check Winsock return codes

* Fix SDL warnings

* Fix sdl warnings for uint64_t

* Fix SDL false positives

* Handle file rename failure.  This was flagged by SDL.

* switched range based for loop to iterator based for loop since SDL check flags uninitalized memory.

* SDL check flagged possible uninitialized variable.

* SDL check flagged clarification.  Make operator preceedence explicit using parenthesis.

* SDL check flagged potential memory leak.  realloc returns null if it fails.

* Preserve current behavior.  vw does not currently throw an exception if rename() fails.

* VW_TRACE is only implemented for Win32.  Writing warnings to cerr if rename() fails.

* Missing endl

* 1) rename() returns 0 on success 2) THROW on rename() error

* 1) OS Invariant rename() using boost 2) change model and cache files so tests can run in parallel

* Rolling back boost changes to reduce dependencies

* Fixed test outputs to match changed model and cache filenames
  • Loading branch information...
rajan-chari authored and JohnLangford committed Nov 27, 2017
1 parent dbe577e commit 127f244095ab9d31df3da267842dba6bd71e27d4
@@ -30,6 +30,9 @@
<ProjectGuid>{E5865596-E5F0-4CA3-B04A-4E34B798744A}</ProjectGuid>
<RootNamespace>c_test</RootNamespace>
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\vowpalwabbit\</SolutionDir>
<!-- This is the ruleset file for code analysis, you can change it in VS -->
<CodeAnalysisRuleSet>..\sdl\SDL-7.0-Recommended.ruleset</CodeAnalysisRuleSet>
<RunCodeAnalysis>true</RunCodeAnalysis>
</PropertyGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Configuration">
@@ -217,4 +220,5 @@
</PropertyGroup>
<Error Condition="!Exists('$(SolutionDir)\.nuget\NuGet.targets')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\.nuget\NuGet.targets'))" />
</Target>
<Import Project="..\sdl\SDL-7.0-NativeAnalysis.targets" />
</Project>
@@ -33,6 +33,9 @@
<ProjectName>spanning_tree</ProjectName>
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\vowpalwabbit\</SolutionDir>
<WindowsTargetPlatformVersion>8.1</WindowsTargetPlatformVersion>
<!-- This is the ruleset file for code analysis, you can change it in VS -->
<CodeAnalysisRuleSet>..\sdl\SDL-7.0-Recommended.ruleset</CodeAnalysisRuleSet>
<RunCodeAnalysis>true</RunCodeAnalysis>
</PropertyGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Configuration">
@@ -268,4 +271,5 @@
</PropertyGroup>
<Error Condition="!Exists('$(SolutionDir)\.nuget\NuGet.targets')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\.nuget\NuGet.targets'))" />
</Target>
<Import Project="..\sdl\SDL-7.0-NativeAnalysis.targets" />
</Project>
@@ -20,6 +20,9 @@
<Keyword>ManagedCProj</Keyword>
<RootNamespace>cs_vld</RootNamespace>
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\vowpalwabbit\</SolutionDir>
<!-- This is the ruleset file for code analysis, you can change it in VS -->
<CodeAnalysisRuleSet>..\..\sdl\SDL-7.0-Recommended.ruleset</CodeAnalysisRuleSet>
<RunCodeAnalysis>true</RunCodeAnalysis>
</PropertyGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'" Label="Configuration">
@@ -148,4 +151,5 @@
<Error Condition="!Exists('..\..\vowpalwabbit\packages\VisualLeakDetector.2.5.0.0\build\native\VisualLeakDetector.targets')" Text="$([System.String]::Format('$(ErrorText)', '..\..\vowpalwabbit\packages\VisualLeakDetector.2.5.0.0\build\native\VisualLeakDetector.targets'))" />
<Error Condition="!Exists('..\..\vowpalwabbit\packages\MSBuildTasks.1.5.0.214\build\MSBuildTasks.targets')" Text="$([System.String]::Format('$(ErrorText)', '..\..\vowpalwabbit\packages\MSBuildTasks.1.5.0.214\build\MSBuildTasks.targets'))" />
</Target>
<Import Project="..\..\sdl\SDL-7.0-NativeAnalysis.targets" />
</Project>
@@ -32,6 +32,8 @@
<Keyword>ManagedCProj</Keyword>
<RootNamespace>vw_clr</RootNamespace>
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\vowpalwabbit\</SolutionDir>
<CodeAnalysisRuleSet>..\..\sdl\SDL-7.0-Recommended.ruleset</CodeAnalysisRuleSet>
<RunCodeAnalysis>true</RunCodeAnalysis>
</PropertyGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Configuration">
@@ -427,4 +429,5 @@
<FileUpdate Files="Resource.rc" Regex="FILEVERSION .*" ReplacementText="FILEVERSION $(VowpalWabbitAssemblyVersion.Replace('.',','))" Encoding="UTF-16" />
<FileUpdate Files="Resource.rc" Regex="PRODUCTVERSION .*" ReplacementText="PRODUCTVERSION $(VowpalWabbitAssemblyVersion.Replace('.',','))" Encoding="UTF-16" />
</Target>
<Import Project="..\..\sdl\SDL-7.0-NativeAnalysis.targets" />
</Project>
@@ -74,20 +74,20 @@ public void TestID()
vw.SaveModel("model");

vw.ID = "def";
vw.SaveModel("model.1");
vw.SaveModel("model.TestID");
}

using (var vw = new VowpalWabbit("-i model"))
{
Assert.AreEqual("abc", vw.ID);
}

using (var vw = new VowpalWabbit("-i model.1"))
using (var vw = new VowpalWabbit("-i model.TestID"))
{
Assert.AreEqual("def", vw.ID);
}

using (var vwm = new VowpalWabbitModel("-i model.1"))
using (var vwm = new VowpalWabbitModel("-i model.TestID"))
{
Assert.AreEqual("def", vwm.ID);
using (var vw = new VowpalWabbit(new VowpalWabbitSettings { Model = vwm }))
@@ -128,7 +128,7 @@ public void TestReload()
using (var vw = new VowpalWabbit(""))
{
vw.ID = "def";
vw.SaveModel("model.1");
vw.SaveModel("model.TestReload");

vw.Reload();

@@ -12,7 +12,7 @@ public class TestWrapper : TestBase
[TestCategory("Vowpal Wabbit")]
public void VwCleanupTest()
{
new VowpalWabbit<Test1>("-k -l 20 --initial_t 128000 --power_t 1 -c --passes 8 --invariant --ngram 3 --skips 1 --holdout_off")
new VowpalWabbit<Test1>("-k -l 20 --initial_t 128000 --power_t 1 -c --cache_file VwCleanupTest.cache --passes 8 --invariant --ngram 3 --skips 1 --holdout_off")
.Dispose();
}

@@ -30,6 +30,9 @@
<ProjectGuid>{D3D9B744-D0FC-4BC7-94A8-89C1AC5692DE}</ProjectGuid>
<RootNamespace>deploy_vw</RootNamespace>
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\vowpalwabbit\</SolutionDir>
<!-- This is the ruleset file for code analysis, you can change it in VS -->
<CodeAnalysisRuleSet>..\sdl\SDL-7.0-Recommended.ruleset</CodeAnalysisRuleSet>
<RunCodeAnalysis>true</RunCodeAnalysis>
</PropertyGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Configuration">
@@ -290,4 +293,5 @@ xcopy /v /i /r /y "$(VCInstallDir)redist\$(PlatformShortName)\Microsoft.VC120.O
</PropertyGroup>
<Error Condition="!Exists('$(SolutionDir)\.nuget\NuGet.targets')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\.nuget\NuGet.targets'))" />
</Target>
<Import Project="..\sdl\SDL-7.0-NativeAnalysis.targets" />
</Project>
@@ -15,6 +15,9 @@
<RootNamespace>python</RootNamespace>
<Keyword>Win32Proj</Keyword>
<ProjectName>python27</ProjectName>
<!-- This is the ruleset file for code analysis, you can change it in VS -->
<CodeAnalysisRuleSet>..\..\sdl\SDL-7.0-Recommended.ruleset</CodeAnalysisRuleSet>
<RunCodeAnalysis>true</RunCodeAnalysis>
</PropertyGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'" Label="Configuration">
@@ -183,4 +186,5 @@
<Error Condition="!Exists('..\..\vowpalwabbit\packages\boost-vc140.1.63.0.0\build\native\boost-vc140.targets')" Text="$([System.String]::Format('$(ErrorText)', '..\..\vowpalwabbit\packages\boost-vc140.1.63.0.0\build\native\boost-vc140.targets'))" />
<Error Condition="!Exists('..\..\vowpalwabbit\packages\zlib.v140.windesktop.msvcstl.static.rt-dyn.1.2.8.8\build\native\zlib.v140.windesktop.msvcstl.static.rt-dyn.targets')" Text="$([System.String]::Format('$(ErrorText)', '..\..\vowpalwabbit\packages\zlib.v140.windesktop.msvcstl.static.rt-dyn.1.2.8.8\build\native\zlib.v140.windesktop.msvcstl.static.rt-dyn.targets'))" />
</Target>
<Import Project="..\..\sdl\SDL-7.0-NativeAnalysis.targets" />
</Project>
@@ -0,0 +1,48 @@
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" InitialTarget="CheckAnalysisSKU" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<Target Name="CheckAnalysisSKU">
<Error Text="SDL code analysis cannot be run on the Visual Studio Express SKU, please use a different version of Visual Studio"
Condition="'$(CodeAnalysisVSSku)'!='Express' and '$(Language)'=='C++' and '$(RunCodeAnalysisOnThisProject)'=='true'" />
</Target>

<!-- To override the location of SDL plugins, set SDLPluginLocationx86 and SDLPluginLocationx64 -->
<!-- The default is the VC install path (where the compiler is found) -->
<PropertyGroup>
<SDLPluginLocationx86 Condition="'$(SDLPluginLocationx86)'=='' and '$(Language)'=='C++' and '$(RunCodeAnalysisOnThisProject)'=='true'">$(VCInstallDir)\bin\</SDLPluginLocationx86>
<SDLPluginLocationx64 Condition="'$(SDLPluginLocationx64)'=='' and '$(Language)'=='C++' and '$(RunCodeAnalysisOnThisProject)'=='true'">$(VCInstallDir)\bin\amd64\</SDLPluginLocationx64>
</PropertyGroup>

<ItemDefinitionGroup Condition="'$(Language)'=='C++' and '$(RunCodeAnalysisOnThisProject)'=='true'">
<ClCompile>
<!-- Configure the location of EnumIndex -->
<PREfastAdditionalPlugins Condition="'$(PreferredToolArchitecture)'=='x64'and Exists('$(SDLPluginLocationx64)\EnumIndex.dll')">$(SDLPluginLocationx64)\EnumIndex.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>
<PREfastAdditionalPlugins Condition="'$(PreferredToolArchitecture)'!='x64'and Exists('$(SDLPluginLocationx86)\EnumIndex.dll')">$(SDLPluginLocationx86)\EnumIndex.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>

<!-- Configure the location of HResult -->
<PREfastAdditionalPlugins Condition="'$(PreferredToolArchitecture)'=='x64'and Exists('$(SDLPluginLocationx64)\HResult.dll')">$(SDLPluginLocationx64)\HResult.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>
<PREfastAdditionalPlugins Condition="'$(PreferredToolArchitecture)'!='x64'and Exists('$(SDLPluginLocationx86)\HResult.dll')">$(SDLPluginLocationx86)\HResult.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>

<!-- Configure the location of ThinCryptESP -->
<PREfastAdditionalPlugins Condition="'$(PreferredToolArchitecture)'=='x64'and Exists('$(SDLPluginLocationx64)\ThinCryptESP.dll')">$(SDLPluginLocationx64)\ThinCryptESP.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>
<PREfastAdditionalPlugins Condition="'$(PreferredToolArchitecture)'!='x64'and Exists('$(SDLPluginLocationx86)\ThinCryptESP.dll')">$(SDLPluginLocationx86)\ThinCryptESP.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>

<!-- Configure the location of UnsafeXML -->
<PREfastAdditionalPlugins Condition="'$(PreferredToolArchitecture)'=='x64'and Exists('$(SDLPluginLocationx64)\UnsafeXML.dll')">$(SDLPluginLocationx64)\UnsafeXML.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>
<PREfastAdditionalPlugins Condition="'$(PreferredToolArchitecture)'!='x64'and Exists('$(SDLPluginLocationx86)\UnsafeXML.dll')">$(SDLPluginLocationx86)\UnsafeXML.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>

<!-- Configure the location of VariantClear -->
<PREfastAdditionalPlugins Condition="'$(PreferredToolArchitecture)'=='x64'and Exists('$(SDLPluginLocationx64)\VariantClear.dll')">$(SDLPluginLocationx64)\VariantClear.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>
<PREfastAdditionalPlugins Condition="'$(PreferredToolArchitecture)'!='x64'and Exists('$(SDLPluginLocationx86)\VariantClear.dll')">$(SDLPluginLocationx86)\VariantClear.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>

<!-- Configure the location of KernelDoubleFetch -->
<PREfastAdditionalPlugins Condition="'$(SDLUseKernelDoubleFetch)'=='true' and '$(PreferredToolArchitecture)'=='x64'and Exists('$(SDLPluginLocationx64)\KernelDoubleFetch.dll')">$(SDLPluginLocationx64)\KernelDoubleFetch.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>
<PREfastAdditionalPlugins Condition="'$(SDLUseKernelDoubleFetch)'=='true' and '$(PreferredToolArchitecture)'!='x64'and Exists('$(SDLPluginLocationx86)\KernelDoubleFetch.dll')">$(SDLPluginLocationx86)\KernelDoubleFetch.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>

<!-- Configure the location of EspX -->
<PREfastAdditionalPlugins Condition="'$(SDLUseEspX)'=='true' and '$(PreferredToolArchitecture)'=='x64'and Exists('$(SDLPluginLocationx64)\EspX.dll')">$(SDLPluginLocationx64)\EspX.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>
<PREfastAdditionalPlugins Condition="'$(SDLUseEspX)'=='true' and '$(PreferredToolArchitecture)'!='x64'and Exists('$(SDLPluginLocationx86)\EspX.dll')">$(SDLPluginLocationx86)\EspX.dll;%(PREfastAdditionalPlugins)</PREfastAdditionalPlugins>

</ClCompile>
</ItemDefinitionGroup>
</Project>
@@ -0,0 +1,128 @@
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="SDL 7.0 Recommended Native Rules" Description="This ruleset contains all the security checks that are recommended for SDL 7.0. Some rules are part of the compiler. For other rules, you need to install the native rules for SDL from codebox." ToolsVersion="11.0">
<Rules AnalyzerId="Microsoft.Analyzers.NativeCodeAnalysis" RuleNamespace="Microsoft.Rules.Native">
<Rule Id="C6001" Action="Warning" />
<Rule Id="C6031" Action="Warning" />
<Rule Id="C6063" Action="Warning" />
<Rule Id="C6064" Action="Warning" />
<Rule Id="C6066" Action="Warning" />
<Rule Id="C6101" Action="Warning" />
<Rule Id="C6217" Action="Warning" />
<Rule Id="C6220" Action="Warning" />
<Rule Id="C6225" Action="Warning" />
<Rule Id="C6226" Action="Warning" />
<Rule Id="C6235" Action="Warning" />
<Rule Id="C6236" Action="Warning" />
<Rule Id="C6237" Action="Warning" />
<Rule Id="C6259" Action="Warning" />
<Rule Id="C6269" Action="Warning" />
<Rule Id="C6278" Action="Warning" />
<Rule Id="C6280" Action="Warning" />
<Rule Id="C6281" Action="Warning" />
<Rule Id="C6282" Action="Warning" />
<Rule Id="C6284" Action="Warning" />
<Rule Id="C6285" Action="Warning" />
<Rule Id="C6286" Action="Warning" />
<Rule Id="C6289" Action="Warning" />
<Rule Id="C6290" Action="Warning" />
<Rule Id="C6292" Action="Warning" />
<Rule Id="C6294" Action="Warning" />
<Rule Id="C6297" Action="Warning" />
<Rule Id="C6298" Action="Warning" />
<Rule Id="C6299" Action="Warning" />
<Rule Id="C6302" Action="Warning" />
<Rule Id="C6303" Action="Warning" />
<Rule Id="C6308" Action="Warning" />
<Rule Id="C6312" Action="Warning" />
<Rule Id="C6314" Action="Warning" />
<Rule Id="C6318" Action="Warning" />
<Rule Id="C6319" Action="Warning" />
<Rule Id="C6320" Action="Warning" />
<Rule Id="C6328" Action="Warning" />
<Rule Id="C6336" Action="Warning" />
<Rule Id="C6384" Action="Warning" />
<Rule Id="C6386" Action="Warning" />
<Rule Id="C6388" Action="Warning" />
<Rule Id="C6500" Action="Warning" />
<Rule Id="C6503" Action="Warning" />
<Rule Id="C6508" Action="Warning" />
<Rule Id="C6511" Action="Warning" />
<Rule Id="C6513" Action="Warning" />
<Rule Id="C6514" Action="Warning" />
<Rule Id="C6516" Action="Warning" />
<Rule Id="C6517" Action="Warning" />
<Rule Id="C6540" Action="Warning" />
<Rule Id="C6551" Action="Warning" />
<Rule Id="C26000" Action="Warning" />
<Rule Id="C26006" Action="Warning" />
<Rule Id="C26007" Action="Warning" />
<Rule Id="C26014" Action="Warning" />
<Rule Id="C26015" Action="Warning" />
<Rule Id="C26016" Action="Warning" />
<Rule Id="C26030" Action="Warning" />
<Rule Id="C26031" Action="Warning" />
<Rule Id="C26053" Action="Warning" />
<Rule Id="C28023" Action="Warning" />
<Rule Id="C28193" Action="Warning" />
<Rule Id="C28250" Action="Warning" />
<Rule Id="C28278" Action="Warning" />
<Rule Id="C28301" Action="Warning" />
<Rule Id="C33004" Action="Warning" />
<Rule Id="C33006" Action="Warning" />
<Rule Id="C33088" Action="Warning" />
<Rule Id="C6029" Action="Error" />
<Rule Id="C6053" Action="Error" />
<Rule Id="C6059" Action="Error" />
<Rule Id="C6067" Action="Error" />
<Rule Id="C6200" Action="Error" />
<Rule Id="C6201" Action="Error" />
<Rule Id="C6214" Action="Error" />
<Rule Id="C6215" Action="Error" />
<Rule Id="C6216" Action="Error" />
<Rule Id="C6230" Action="Error" />
<Rule Id="C6248" Action="Error" />
<Rule Id="C6260" Action="Error" />
<Rule Id="C6276" Action="Error" />
<Rule Id="C6277" Action="Error" />
<Rule Id="C6279" Action="Error" />
<Rule Id="C6293" Action="Error" />
<Rule Id="C6305" Action="Error" />
<Rule Id="C6306" Action="Error" />
<Rule Id="C6317" Action="Error" />
<Rule Id="C6324" Action="Error" />
<Rule Id="C6334" Action="Error" />
<Rule Id="C6383" Action="Error" />
<Rule Id="C6504" Action="Error" />
<Rule Id="C6505" Action="Error" />
<Rule Id="C6506" Action="Error" />
<Rule Id="C6509" Action="Error" />
<Rule Id="C6510" Action="Error" />
<Rule Id="C6515" Action="Error" />
<Rule Id="C6518" Action="Error" />
<Rule Id="C28208" Action="Error" />
<Rule Id="C28218" Action="Error" />
<Rule Id="C28220" Action="Error" />
<Rule Id="C28222" Action="Error" />
<Rule Id="C28230" Action="Error" />
<Rule Id="C28241" Action="Error" />
<Rule Id="C28243" Action="Error" />
<Rule Id="C28285" Action="Error" />
<Rule Id="C28290" Action="Error" />
<Rule Id="C28309" Action="Error" />
<Rule Id="C33001" Action="Error" />
<Rule Id="C33005" Action="Error" />
<Rule Id="C33010" Action="Error" />
<Rule Id="C33020" Action="Error" />
<Rule Id="C33087" Action="Error" />
<Rule Id="C33089" Action="Error" />
<Rule Id="C33090" Action="Warning" />
<Rule Id="C33091" Action="Error" />
<Rule Id="C33092" Action="Error" />
<Rule Id="C33093" Action="Error" />
<Rule Id="C33095" Action="Error" />
<Rule Id="C33096" Action="Error" />
<Rule Id="C33097" Action="Error" />
<Rule Id="C33098" Action="Error" />
<Rule Id="C33099" Action="Error" />
</Rules>
</RuleSet>
Oops, something went wrong.

0 comments on commit 127f244

Please sign in to comment.