Skip to content

Commit 2b82f03

Browse files
samuel-lee-msftsamuel-lee-msft
committed
Merged PR 11375621: Fix outstanding Prefast warnings
+ Fixing various Prefast warnings to get us clean w.r.t. Prefast + Enable Prefast failures to break PR builds + Reduce noisy build warnings + Unpin Windows container images as using old images Related work items: #52514550, #52514551, #52514554, #52514555, #52514556, #52514557, #52514559, #52514560, #52514561, #52514562, #52514632, #52514633, #52514634, #53004108, #53004109, #53130817
1 parent 241eff3 commit 2b82f03

30 files changed

+318
-298
lines changed

‎.pipelines/OneBranch.Official.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ schedules:
2929
variables:
3030
CDP_DEFINITION_BUILD_COUNT: $[counter('', 0)] # needed for onebranch.pipeline.version task https://aka.ms/obpipelines/versioning
3131
LinuxContainerImage: 'onebranch.azurecr.io/linux/ubuntu-2004:latest' # Docker image which is used to build the project https://aka.ms/obpipelines/containers
32-
WindowsContainerImage: 'onebranch.azurecr.io/windows/ltsc2019/vse2022@sha256:a3083215a4675bad774ec88005dcf57436b3423584d0db3e82b6de3f780213ba'
32+
WindowsContainerImage: 'onebranch.azurecr.io/windows/ltsc2019/vse2022:latest'
3333

3434
resources:
3535
repositories:

‎.pipelines/OneBranch.WindowsUndocked.Official.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ parameters:
5858

5959
variables:
6060
# https://aka.ms/obpipelines/containers
61-
WindowsContainerImage: 'onebranch.azurecr.io/windows/ltsc2022/vse2022@sha256:394ae836d8bbb5f5f7659744b85796ac823ab1410527ac26d143837bdecbde2a'
61+
WindowsContainerImage: 'onebranch.azurecr.io/windows/ltsc2022/vse2022:latest'
6262

6363
resources:
6464
repositories:

‎.pipelines/OneBranch.WindowsUndocked.PullRequest.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pr:
1919

2020
variables:
2121
# https://aka.ms/obpipelines/containers
22-
WindowsContainerImage: 'onebranch.azurecr.io/windows/ltsc2022/vse2022@sha256:394ae836d8bbb5f5f7659744b85796ac823ab1410527ac26d143837bdecbde2a'
22+
WindowsContainerImage: 'onebranch.azurecr.io/windows/ltsc2022/vse2022:latest'
2323

2424
resources:
2525
repositories:

‎.pipelines/templates/build-windows-undocked.yml‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ parameters:
88
config: 'Debug,Release'
99
platform: 'x86,x64,arm64'
1010
nativeCompiler: true
11-
buildType: 'private'
11+
buildType: 'pr'
1212
sign: false # Only signs UM binaries, for external (to Windows repo) release
1313
# Packaging args
1414
package: false
@@ -94,10 +94,15 @@ jobs:
9494
ob_outputDirectory: $(Build.SourcesDirectory)\build\bin\$(ob_build_platform_win)$(ob_build_config_win)
9595
ob_artifactSuffix: _$(ob_build_platform_win)$(ob_build_config_win)
9696
# https://aka.ms/obpipelines/sdl
97-
ob_sdl_tsa_enabled: true # When TSA is disabled all SDL tools will forced into 'break' build mode.
97+
# When TSA is enabled bugs are filed on SDL errors. When TSA is disabled, most SDL tools break the build.
98+
# Make official builds file bugs but PR builds just break the build.
99+
${{ if eq(parameters.buildType, 'official') }}:
100+
ob_sdl_tsa_enabled: true
101+
${{ if eq(parameters.buildType, 'pr') }}:
102+
ob_sdl_tsa_enabled: false
98103
ob_sdl_binskim_break: true
99104
ob_sdl_policheck_break: true
100-
ob_sdl_prefast_break: false
105+
ob_sdl_prefast_break: true
101106
${{ if eq(parameters.sign, true) }}:
102107
ob_sdl_codeSignValidation_excludes: -|**\*.sys # Signing is not supported for KM drivers
103108
${{ if eq(parameters.sign, false) }}:
@@ -137,7 +142,7 @@ jobs:
137142
userProvideBuildInfo: auto
138143
rulesetName: Custom
139144
customRuleset: '$(Build.SourcesDirectory)\tvs.ruleset'
140-
excludedPaths: 'c:/program files (x86)/windows kits/'
145+
excludedPaths: 'c:/program files (x86)/windows kits/#c:/__w/1/s/unittest/symcryptdependencies/inc/'
141146
env:
142147
SYSTEM_ACCESSTOKEN: $(System.AccessToken)
143148

‎inc/symcrypt.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8511,7 +8511,7 @@ SymCryptXmsskeyGenerate(
85118511
SYMCRYPT_ERROR
85128512
SYMCRYPT_CALL
85138513
SymCryptXmsskeySetValue(
8514-
_In_reads_bytes_( cbSrc ) PCBYTE pbInput,
8514+
_In_reads_bytes_( cbInput ) PCBYTE pbInput,
85158515
SIZE_T cbInput,
85168516
SYMCRYPT_XMSSKEY_TYPE keyType,
85178517
UINT32 flags,
@@ -8583,11 +8583,11 @@ SymCryptXmsskeySetValue(
85838583
SYMCRYPT_ERROR
85848584
SYMCRYPT_CALL
85858585
SymCryptXmsskeyGetValue(
8586-
_In_ PCSYMCRYPT_XMSS_KEY pKey,
8587-
SYMCRYPT_XMSSKEY_TYPE keyType,
8588-
UINT32 flags,
8589-
_Out_writes_bytes_( cbKey ) PBYTE pbOutput,
8590-
SIZE_T cbOutput );
8586+
_In_ PCSYMCRYPT_XMSS_KEY pKey,
8587+
SYMCRYPT_XMSSKEY_TYPE keyType,
8588+
UINT32 flags,
8589+
_Out_writes_bytes_( cbOutput ) PBYTE pbOutput,
8590+
SIZE_T cbOutput );
85918591
//
85928592
// Get public/private key value from an XMSS/XMSS^MT key object
85938593
//

‎inc/symcrypt_internal.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2100,6 +2100,14 @@ typedef const SYMCRYPT_ECPOINT * PCSYMCRYPT_ECPOINT;
21002100

21012101
#define SYMCRYPT_BYTES_FROM_BITS(bits) ( ( (bits) + 7 ) / 8 )
21022102

2103+
// The maximum number of bits in any integer value that the library supports. If the
2104+
// caller's input exceed this bound then the the integer object will not be created.
2105+
// The caller either must ensure the bound is not exceeded, or check for NULL before
2106+
// using created SymCrypt objects.
2107+
// The primary purpose of this limit is to avoid integer overlows in size computations.
2108+
// Having a reasonable upper bound avoids all size overflows, even on 32-bit CPUs
2109+
#define SYMCRYPT_INT_MAX_BITS ((UINT32)(1 << 20))
2110+
21032111
//
21042112
// Upper bound for the number of digits: this MUST be enforced on runtime
21052113
// on all Allocate, SizeOf, and Create calls which take as input a digit number.

‎inc/symcrypt_low_level.h‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -273,14 +273,6 @@ a ModElement object.
273273
// General functions for integers
274274
//
275275

276-
// The maximum number of bits in any integer value that the library supports. If the
277-
// caller's input exceed this bound then the the integer object will not be created.
278-
// The caller either must ensure the bound is not exceeded, or check for NULL before
279-
// using created SymCrypt objects.
280-
// The primary purpose of this limit is to avoid integer overlows in size computations.
281-
// Having a reasonable upper bound avoids all size overflows, even on 32-bit CPUs
282-
#define SYMCRYPT_INT_MAX_BITS ((UINT32)(1 << 20))
283-
284276
UINT32
285277
SymCryptDigitsFromBits( UINT32 nBits );
286278
//

‎lib/aes-asm.c‎

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,12 @@ SymCryptAesEcbEncryptAsm(
1515
_Out_writes_( cbData ) PBYTE pbDst,
1616
SIZE_T cbData )
1717
{
18-
SIZE_T cbToDo = cbData & ~(SYMCRYPT_AES_BLOCK_SIZE - 1);
19-
SIZE_T i;
20-
21-
//
22-
// This loop condition is slightly strange.
23-
// If I use i < cbToDo (which is correct) then Prefast complains about buffer overflows.
24-
// Even using SYMCRYPT_ASSERT which does an _Analysis_assume_ I can't fix the Prefast issue.
25-
// The +15 in the code is slightly slower but it solves the Prefast issue.
26-
//
27-
28-
for( i=0; (i+SYMCRYPT_AES_BLOCK_SIZE-1) < cbToDo; i+= SYMCRYPT_AES_BLOCK_SIZE )
18+
while( cbData >= SYMCRYPT_AES_BLOCK_SIZE )
2919
{
30-
SymCryptAesEncryptAsm( pExpandedKey, pbSrc + i, pbDst + i );
20+
SymCryptAesEncryptAsm( pExpandedKey, pbSrc, pbDst );
21+
pbSrc += SYMCRYPT_AES_BLOCK_SIZE;
22+
pbDst += SYMCRYPT_AES_BLOCK_SIZE;
23+
cbData -= SYMCRYPT_AES_BLOCK_SIZE;
3124
}
3225
}
3326

@@ -39,18 +32,11 @@ SymCryptAesEcbDecryptAsm(
3932
_Out_writes_( cbData ) PBYTE pbDst,
4033
SIZE_T cbData )
4134
{
42-
SIZE_T cbToDo = cbData & ~(SYMCRYPT_AES_BLOCK_SIZE - 1);
43-
SIZE_T i;
44-
45-
//
46-
// This loop condition is slightly strange.
47-
// If I use i < cbToDo (which is correct) then Prefast complains about buffer overflows.
48-
// Even using SYMCRYPT_ASSERT which does an _Analysis_assume_ I can't fix the Prefast issue.
49-
// The +15 in the code is slightly slower but it solves the Prefast issue.
50-
//
51-
52-
for( i=0; (i+SYMCRYPT_AES_BLOCK_SIZE-1) < cbToDo; i+= SYMCRYPT_AES_BLOCK_SIZE )
35+
while( cbData >= SYMCRYPT_AES_BLOCK_SIZE )
5336
{
54-
SymCryptAesDecryptAsm( pExpandedKey, pbSrc + i, pbDst + i );
37+
SymCryptAesDecryptAsm( pExpandedKey, pbSrc, pbDst );
38+
pbSrc += SYMCRYPT_AES_BLOCK_SIZE;
39+
pbDst += SYMCRYPT_AES_BLOCK_SIZE;
40+
cbData -= SYMCRYPT_AES_BLOCK_SIZE;
5541
}
5642
}

‎lib/aes-c.c‎

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -364,19 +364,12 @@ SymCryptAesEcbEncryptC(
364364
_Out_writes_( cbData ) PBYTE pbDst,
365365
SIZE_T cbData )
366366
{
367-
SIZE_T cbToDo = cbData & ~(SYMCRYPT_AES_BLOCK_SIZE - 1);
368-
SIZE_T i;
369-
370-
//
371-
// This loop condition is slightly strange.
372-
// If I use i < cbToDo (which is correct) then Prefast complains about buffer overflows.
373-
// Even using SYMCRYPT_ASSERT which does an _Analysis_assume_ I can't fix the Prefast issue.
374-
// The +15 in the code is slightly slower but it solves the Prefast issue.
375-
//
376-
377-
for( i=0; (i+SYMCRYPT_AES_BLOCK_SIZE-1) < cbToDo; i+= SYMCRYPT_AES_BLOCK_SIZE )
367+
while( cbData >= SYMCRYPT_AES_BLOCK_SIZE )
378368
{
379-
SymCryptAesEncryptC( pExpandedKey, pbSrc + i, pbDst + i );
369+
SymCryptAesEncryptC( pExpandedKey, pbSrc, pbDst );
370+
pbSrc += SYMCRYPT_AES_BLOCK_SIZE;
371+
pbDst += SYMCRYPT_AES_BLOCK_SIZE;
372+
cbData -= SYMCRYPT_AES_BLOCK_SIZE;
380373
}
381374
}
382375

@@ -388,19 +381,12 @@ SymCryptAesEcbDecryptC(
388381
_Out_writes_( cbData ) PBYTE pbDst,
389382
SIZE_T cbData )
390383
{
391-
SIZE_T cbToDo = cbData & ~(SYMCRYPT_AES_BLOCK_SIZE - 1);
392-
SIZE_T i;
393-
394-
//
395-
// This loop condition is slightly strange.
396-
// If I use i < cbToDo (which is correct) then Prefast complains about buffer overflows.
397-
// Even using SYMCRYPT_ASSERT which does an _Analysis_assume_ I can't fix the Prefast issue.
398-
// The +15 in the code is slightly slower but it solves the Prefast issue.
399-
//
400-
401-
for( i=0; (i+SYMCRYPT_AES_BLOCK_SIZE-1) < cbToDo; i+= SYMCRYPT_AES_BLOCK_SIZE )
384+
while( cbData >= SYMCRYPT_AES_BLOCK_SIZE )
402385
{
403-
SymCryptAesDecryptC( pExpandedKey, pbSrc + i, pbDst + i );
386+
SymCryptAesDecryptC( pExpandedKey, pbSrc, pbDst );
387+
pbSrc += SYMCRYPT_AES_BLOCK_SIZE;
388+
pbDst += SYMCRYPT_AES_BLOCK_SIZE;
389+
cbData -= SYMCRYPT_AES_BLOCK_SIZE;
404390
}
405391
}
406392

‎lib/aes-default.c‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -303,19 +303,12 @@ SymCryptAesEcbDecrypt(
303303
_Out_writes_( cbData ) PBYTE pbDst,
304304
SIZE_T cbData )
305305
{
306-
SIZE_T cbToDo = cbData & ~(SYMCRYPT_AES_BLOCK_SIZE - 1);
307-
SIZE_T i;
308-
309-
//
310-
// This loop condition is slightly strange.
311-
// If I use i < cbToDo (which is correct) then Prefast complains about buffer overflows.
312-
// Even using SYMCRYPT_ASSERT which does an _Analysis_assume_ I can't fix the Prefast issue.
313-
// The +15 in the code is slightly slower but it solves the Prefast issue.
314-
//
315-
316-
for( i=0; (i+SYMCRYPT_AES_BLOCK_SIZE-1) < cbToDo; i+= SYMCRYPT_AES_BLOCK_SIZE )
306+
while( cbData >= SYMCRYPT_AES_BLOCK_SIZE )
317307
{
318-
SymCryptAesDecrypt( pExpandedKey, pbSrc + i, pbDst + i );
308+
SymCryptAesDecrypt( pExpandedKey, pbSrc, pbDst );
309+
pbSrc += SYMCRYPT_AES_BLOCK_SIZE;
310+
pbDst += SYMCRYPT_AES_BLOCK_SIZE;
311+
cbData -= SYMCRYPT_AES_BLOCK_SIZE;
319312
}
320313
}
321314

0 commit comments

Comments
 (0)