Skip to content

Commit 4aea8e3

Browse files
BrennanConroyBrennanConroy
committed
more
1 parent aeb96d2 commit 4aea8e3

File tree

10 files changed

+76
-26
lines changed

10 files changed

+76
-26
lines changed

‎src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<Project DefaultTargets="Build" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3-
<Import Project="..\..\build\Build.Lib.Settings" />
43
<PropertyGroup Label="Globals">
54
<ProjectGuid>{55494E58-E061-4C4C-A0A8-837008E72F85}</ProjectGuid>
65
<RootNamespace>NewCommon</RootNamespace>
7-
<RunCodeAnalysis>false</RunCodeAnalysis>
86
</PropertyGroup>
7+
<Import Project="..\..\build\Build.Lib.Settings" />
98
<ItemDefinitionGroup>
109
<ClCompile>
1110
<AdditionalIncludeDirectories>..\iislib;$(LibNetHostPath)</AdditionalIncludeDirectories>

‎src/Servers/IIS/AspNetCoreModuleV2/CommonLib/RegistryKey.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ std::optional<DWORD> RegistryKey::TryGetDWORD(HKEY section, const std::wstring&
1818

1919
std::optional<std::wstring> RegistryKey::TryGetString(HKEY section, const std::wstring& subSectionName, const std::wstring& valueName)
2020
{
21-
DWORD cbData;
21+
DWORD cbData{};
2222

2323
if (!CheckReturnValue(RegGetValue(section, subSectionName.c_str(), valueName.c_str(), RRF_RT_REG_SZ, nullptr, nullptr, &cbData)))
2424
{

‎src/Servers/IIS/AspNetCoreModuleV2/CommonLib/StandardStreamRedirection.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,8 @@ void StandardStreamRedirection::Stop()
156156
dwThreadStatus == STILL_ACTIVE)
157157
{
158158
LOG_WARN(L"Thread reading stdout/err hit timeout, forcibly closing thread.");
159+
// Using TerminateThread does not allow proper thread clean up.
160+
#pragma warning(suppress: 6258)
159161
TerminateThread(m_hErrThread, STATUS_CONTROL_C_EXIT);
160162
}
161163
}

‎src/Servers/IIS/AspNetCoreModuleV2/CommonLib/debugutil.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ std::wstring
114114
GetModuleName()
115115
{
116116
WCHAR path[MAX_PATH];
117-
LOG_LAST_ERROR_IF(!GetModuleFileName(g_hModule, path, sizeof(path)));
117+
LOG_LAST_ERROR_IF(!GetModuleFileName(g_hModule, path, sizeof(path) / sizeof(WCHAR)));
118118
return path;
119119
}
120120

‎src/Servers/IIS/AspNetCoreModuleV2/DefaultRules.ruleset‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212
<Rule Id="C26116" Action="Error" />
1313
<Rule Id="C26117" Action="Error" />
1414
<Rule Id="C26130" Action="Error" />
15-
<Rule Id="C26135" Action="Error" />
15+
<Rule Id="C26135" Action="None" /> <!-- Missing annotation <annotation> at function '<func>' -->
1616
<Rule Id="C26140" Action="Error" />
17-
<Rule Id="C26160" Action="Error" />
17+
<Rule Id="C26160" Action="None" /> <!-- Caller possibly failing to hold lock '<lock>' before calling function '<func>'. -->
1818
<Rule Id="C26165" Action="Error" />
1919
<Rule Id="C26166" Action="Error" />
2020
<Rule Id="C26167" Action="Error" />
@@ -38,7 +38,7 @@
3838
<Rule Id="C26426" Action="Error" />
3939
<Rule Id="C26427" Action="Error" />
4040
<Rule Id="C26429" Action="None" /> <!-- Symbol is never tested for nullness, it can be marked as gsl::not_null. -->
41-
<Rule Id="C26430" Action="Error" />
41+
<Rule Id="C26430" Action="None" /> <!-- Symbol '<var>' is not tested for nullness on all paths -->
4242
<Rule Id="C26431" Action="Error" />
4343
<Rule Id="C26432" Action="None" /> <!-- If you define or delete any default operation in the type 'type-name', define or delete them all -->
4444
<Rule Id="C26433" Action="None" /> <!-- Function should be marked with override -->
@@ -58,23 +58,24 @@
5858
<Rule Id="C26448" Action="None" /> <!-- Consider using gsl::finally if final action is intended -->
5959
<Rule Id="C26449" Action="Error" />
6060
<Rule Id="C26450" Action="Error" />
61-
<Rule Id="C26451" Action="Error" />
61+
<Rule Id="C26451" Action="None" /> <!-- Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow -->
6262
<Rule Id="C26452" Action="Error" />
6363
<Rule Id="C26453" Action="Error" />
6464
<Rule Id="C26454" Action="Error" />
6565
<Rule Id="C26455" Action="None" /> <!-- Default constructor should not throw. Declare it 'noexcept' -->
66+
<Rule Id="C26457" Action="None" /> <!-- (void) should not be used to ignore return values, use 'std::ignore =' instead -->
6667
<Rule Id="C26459" Action="Error" />
6768
<Rule Id="C26460" Action="None" /> <!-- The reference argument 'argument' for function 'function' can be marked as const -->
6869
<Rule Id="C26461" Action="None" /> <!-- The pointer argument 'argument' for function 'function' can be marked as a pointer to const -->
69-
<Rule Id="C26462" Action="Error" />
70+
<Rule Id="C26462" Action="None" /> <!-- The value pointed to by '<var>' is assigned only once, mark it as a pointer to const -->
7071
<Rule Id="C26463" Action="Error" />
7172
<Rule Id="C26464" Action="Error" />
7273
<Rule Id="C26465" Action="Error" />
7374
<Rule Id="C26466" Action="Error" />
7475
<Rule Id="C26467" Action="None" /> <!-- Converting from floating point to unsigned integral types results in non-portable code if the double/float has a negative value. -->
7576
<Rule Id="C26471" Action="None" /> <!-- Don't use reinterpret_cast. A cast from void* can use static_cast -->
7677
<Rule Id="C26472" Action="None" /> <!-- Don't use a static_cast for arithmetic conversions. Use brace initialization, gsl::narrow_cast, or gsl::narrow. -->
77-
<Rule Id="C26473" Action="Error" />
78+
<Rule Id="C26473" Action="None" /> <!-- Don't cast between pointer types where the source type and the target type are the same -->
7879
<Rule Id="C26474" Action="Error" />
7980
<Rule Id="C26475" Action="Error" />
8081
<Rule Id="C26476" Action="None" /> <!-- Expression/symbol 'name' uses a naked union 'union' with multiple type pointers: Use variant instead -->
@@ -99,6 +100,7 @@
99100
<Rule Id="C26818" Action="None" /> <!-- Switch statement does not cover all cases. Consider adding a 'default' label -->
100101
<Rule Id="C26819" Action="Error" />
101102
<Rule Id="C26826" Action="None" /> <!-- Don't use C-style variable arguments -->
103+
<Rule Id="C26859" Action="None" /> <!-- Empty optional '<var>' is unwrapped, will throw exception. -->
102104
<Rule Id="C28020" Action="None" /> <!-- The expression 'expr' is not true at this call -->
103105
<Rule Id="C28021" Action="Error" />
104106
<Rule Id="C28022" Action="Error" />
@@ -220,8 +222,8 @@
220222
<Rule Id="C28246" Action="Error" />
221223
<Rule Id="C28250" Action="Error" />
222224
<Rule Id="C28251" Action="None" /> <!-- Inconsistent annotation for function: this instance has an error -->
223-
<Rule Id="C28252" Action="Error" />
224-
<Rule Id="C28253" Action="Error" />
225+
<Rule Id="C28252" Action="None" /> <!-- Inconsistent annotation for '<func>': <param> has '<annotation>' on the prior instance. -->
226+
<Rule Id="C28253" Action="None" /> <!-- Inconsistent annotation for '<func>': <param> has '<annotations>' on this instance -->
225227
<Rule Id="C28254" Action="Error" />
226228
<Rule Id="C28260" Action="Error" />
227229
<Rule Id="C28262" Action="Error" />
@@ -294,7 +296,7 @@
294296
<Rule Id="C6029" Action="Error" />
295297
<Rule Id="C6031" Action="None" /> <!-- Return value ignored: '<func>'. -->
296298
<Rule Id="C6053" Action="Error" />
297-
<Rule Id="C6054" Action="Error" />
299+
<Rule Id="C6054" Action="None" /> <!-- String '<var>' might not be zero-terminated. -->
298300
<Rule Id="C6059" Action="Error" />
299301
<Rule Id="C6063" Action="Error" />
300302
<Rule Id="C6064" Action="Error" />
@@ -396,7 +398,7 @@
396398
<Rule Id="C6385" Action="Error" />
397399
<Rule Id="C6386" Action="Error" />
398400
<Rule Id="C6387" Action="None" /> <!-- '<expr>' could be '<val>': this does not adhere to the specification for the function '<func>' -->
399-
<Rule Id="C6388" Action="Error" />
401+
<Rule Id="C6388" Action="None" /> <!-- '<var>' might not be '<val>': this does not adhere to the specification for the function '<func>' -->
400402
<Rule Id="C6400" Action="Error" />
401403
<Rule Id="C6401" Action="Error" />
402404
<Rule Id="C6411" Action="Error" />

‎src/Servers/IIS/AspNetCoreModuleV2/IISLib/IISLib.vcxproj‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
<ProjectGuid>{09D9D1D6-2951-4E14-BC35-76A23CF9391A}</ProjectGuid>
66
<RootNamespace>IISLib</RootNamespace>
77
<ProjectName>IISLib</ProjectName>
8-
<RunCodeAnalysis>false</RunCodeAnalysis>
98
</PropertyGroup>
109
<PropertyGroup Condition="'$(Configuration)'=='Release'" Label="Configuration">
1110
<WholeProgramOptimization>true</WholeProgramOptimization>

‎src/Servers/IIS/AspNetCoreModuleV2/IISLib/base64.cpp‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,12 @@ Return Values:
5353
*pcchEncoded = cchEncoded;
5454
}
5555

56-
if (cchEncodedStringSize == 0 && pszEncodedString == NULL) {
56+
if (pszEncodedString == NULL) {
5757
return ERROR_SUCCESS;
5858
}
5959

60+
*pszEncodedString = 0;
61+
6062
if (cchEncodedStringSize < cchEncoded) {
6163
// Given buffer is too small to hold encoded string.
6264
return ERROR_INSUFFICIENT_BUFFER;
@@ -66,8 +68,16 @@ Return Values:
6668
ib = ich = 0;
6769
while (ib < cbDecodedBufferSize) {
6870
b0 = pbDecodedBuffer[ib++];
69-
b1 = (ib < cbDecodedBufferSize) ? pbDecodedBuffer[ib++] : 0;
70-
b2 = (ib < cbDecodedBufferSize) ? pbDecodedBuffer[ib++] : 0;
71+
b1 = 0;
72+
b2 = 0;
73+
if (ib < cbDecodedBufferSize)
74+
{
75+
b1 = pbDecodedBuffer[ib++];
76+
}
77+
if (ib < cbDecodedBufferSize)
78+
{
79+
b2 = pbDecodedBuffer[ib++];
80+
}
7181

7282
//
7383
// The checks below for buffer overflow seems redundant to me.
@@ -176,11 +186,20 @@ constexpr auto NA = (255);
176186
BYTE b0, b1, b2, b3;
177187
BYTE * pbDecodeBuffer = (BYTE *) pDecodeBuffer;
178188

179-
cchEncodedSize = (DWORD)wcslen(pszEncodedString);
180-
if (NULL != pcbDecoded) {
189+
if (NULL != pcbDecoded)
190+
{
181191
*pcbDecoded = 0;
182192
}
183193

194+
if (pDecodeBuffer == NULL)
195+
{
196+
return ERROR_SUCCESS;
197+
}
198+
199+
*pbDecodeBuffer = 0;
200+
201+
cchEncodedSize = (DWORD)wcslen(pszEncodedString);
202+
184203
if ((0 == cchEncodedSize) || (0 != (cchEncodedSize % 4))) {
185204
// Input string is not sized correctly to be base64.
186205
return ERROR_INVALID_PARAMETER;
@@ -292,10 +311,12 @@ Return Values:
292311
*pcchEncoded = cchEncoded;
293312
}
294313

295-
if (cchEncodedStringSize == 0 && pszEncodedString == NULL) {
314+
if (pszEncodedString == NULL) {
296315
return ERROR_SUCCESS;
297316
}
298317

318+
*pszEncodedString = 0;
319+
299320
if (cchEncodedStringSize < cchEncoded) {
300321
// Given buffer is too small to hold encoded string.
301322
return ERROR_INSUFFICIENT_BUFFER;
@@ -305,8 +326,16 @@ Return Values:
305326
ib = ich = 0;
306327
while (ib < cbDecodedBufferSize) {
307328
b0 = pbDecodedBuffer[ib++];
308-
b1 = (ib < cbDecodedBufferSize) ? pbDecodedBuffer[ib++] : 0;
309-
b2 = (ib < cbDecodedBufferSize) ? pbDecodedBuffer[ib++] : 0;
329+
b1 = 0;
330+
b2 = 0;
331+
if (ib < cbDecodedBufferSize)
332+
{
333+
b1 = pbDecodedBuffer[ib++];
334+
}
335+
if (ib < cbDecodedBufferSize)
336+
{
337+
b2 = pbDecodedBuffer[ib++];
338+
}
310339

311340
//
312341
// The checks below for buffer overflow seems redundant to me.
@@ -415,11 +444,18 @@ Return Values:
415444
BYTE b0, b1, b2, b3;
416445
BYTE * pbDecodeBuffer = (BYTE *) pDecodeBuffer;
417446

418-
cchEncodedSize = (DWORD)strlen(pszEncodedString);
419447
if (NULL != pcbDecoded) {
420448
*pcbDecoded = 0;
421449
}
422450

451+
if (pDecodeBuffer == NULL) {
452+
return ERROR_SUCCESS;
453+
}
454+
455+
*pbDecodeBuffer = 0;
456+
457+
cchEncodedSize = (DWORD)strlen(pszEncodedString);
458+
423459
if ((0 == cchEncodedSize) || (0 != (cchEncodedSize % 4))) {
424460
// Input string is not sized correctly to be base64.
425461
return ERROR_INVALID_PARAMETER;

‎src/Servers/IIS/AspNetCoreModuleV2/IISLib/stringa.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@ STRA::QueryStr(
135135
return m_Buff.QueryPtr();
136136
}
137137

138+
VOID STRA::EnsureNullTerminated()
139+
{
140+
// m_cchLen represents the strings length, not the underlying buffer length
141+
m_Buff.QueryPtr()[m_cchLen] = '\0';
142+
}
143+
138144
VOID
139145
STRA::Reset(
140146
)
@@ -695,7 +701,7 @@ Return Value:
695701

696702
_ASSERTE( pch );
697703

698-
while (pch[i] != NULL)
704+
while ((DWORD)i < m_cchLen && pch[i] != NULL)
699705
{
700706
//
701707
// Escape characters that are in the non-printable range
@@ -1153,7 +1159,7 @@ STRA::AuxAppendW(
11531159
// ensure we're still NULL terminated in the right spot
11541160
// (regardless of success or failure)
11551161
//
1156-
QueryStr()[m_cchLen] = '\0';
1162+
EnsureNullTerminated();
11571163

11581164
return hr;
11591165
}

‎src/Servers/IIS/AspNetCoreModuleV2/IISLib/stringa.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,9 @@ class STRA
143143
QueryStr(
144144
) const;
145145

146+
147+
VOID EnsureNullTerminated();
148+
146149
VOID
147150
Reset(
148151
);

‎src/Servers/IIS/AspNetCoreModuleV2/IISLib/util.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ Return Values:
4747
// we need to change it to Win32 from ("\\?\")
4848
//
4949

50+
// Buffer overrun while writing to 'pstrPath->QueryStr()'
51+
// We know it's not an overrun because we just copied pszName into pstrPath and pszName is at least 4 in length
52+
#pragma warning(suppress: 6386)
5053
pstrPath->QueryStr()[2] = L'?';
5154
}
5255

0 commit comments

Comments
 (0)