Skip to content

Commit 5ae15ef

Browse files
committed
Add updates to 6101 6029 and 28213
1 parent 01d6ccd commit 5ae15ef

3 files changed

Lines changed: 70 additions & 46 deletions

File tree

docs/code-quality/c28213.md

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,24 @@ title: Warning C28213
44
ms.date: 09/08/2022
55
f1_keywords: ["C28213", "BAD_USEHEADER", "__WARNING_BAD_USEHEADER"]
66
helpviewer_keywords: ["C28213"]
7-
ms.assetid: e141a12a-4c46-47eb-aa9d-a6444472cfaa
87
---
98
# Warning C28213
109

1110
> The `_Use_decl_annotations_` annotation must be used to reference, without modification, a prior declaration.
1211
1312
## Remarks
1413

15-
`_Use_decl_annotations_` tells the compiler to use the annotations from an earlier declaration of the function. If no earlier declaration can be found, or if the current declaration makes changes to the annotations, then this warning is emitted.
14+
`_Use_decl_annotations_` tells the compiler to use the annotations from an earlier declaration of the function. If no earlier declaration can be found, or if the current declaration makes changes to the annotations, then this warning is emitted. `_Use_decl_annotations_` also allows removing all other annotations from the definition and will use the declaration annotations for analysis of the function.
15+
16+
This diagnostic is frequently a side effect of refactoring or fixing other warnings by adjusting the annotations on a function. If this is the case then to fix the issue, the 'correct' set of annotations needs to replace the annotations at the other locations. To determine the correct set of annotations, look at the behavior in the function definition. In most cases this behavior is intended and should be reflected in the annotations to the function. Refer to the guide on [Using SAL Annotations to reduce code defects](using-sal-annotations-to-reduce-c-cpp-code-defects.md) for more information on SAL.
17+
18+
It is important for the annotations to match between the declarations and the definition of a function. When analyzing the call site of the function the declaration annotations are used, so when the declaration and definition do not match it leads to incorrect results from the static analysis tools. When fixing this warning, it is common for it to have cascading effects as the tool reanalyzes the source with updated information.
19+
20+
If this diagnostic is a result of no previous declaration being found in the translation unit then there is most likely a missing `#include`. If the header file is intentionally not included then to resolve this issue you can verify the the annotations in the declaration and definition match and remove `_Use_decl_annotations_`. Be careful when doing this as the two sets of annotations may get out of sync in the future.
1621

1722
Code analysis name: BAD_USEHEADER
1823

19-
## Example
24+
## Examples
2025

2126
The following code generates C28160. The `buffer` parameter annotation doesn't match between the two files.
2227

@@ -36,7 +41,9 @@ void example_func(_Out_writes_z_(n) char* buffer, int n)
3641
}
3742
```
3843

39-
This issue can be fixed by either changing the annotation so they match at all locations, or by removing all annotations except `_Use_decl_annotations_` from the function definition. In this example, `_Out_writes_z_` appears to be correct so we'll move that to the function declaration in the header file. The following code resolves this warning:
44+
This issue can be fixed by either changing the annotation so they match at all locations, or by removing all annotations except `_Use_decl_annotations_` from the function definition after verifying the declaration annotations are correct. In this simple example, `_Out_writes_z_` appears to be correct so we'll move that annotation to the function declaration in the header file. We will also remove all other annotations from the definition to simplify future maintenance although this is optional.
45+
46+
In real world code it is usually not as clear which annotation is correct, consult with the documentation on [using SAL Annotations to reduce code defects](using-sal-annotations-to-reduce-c-cpp-code-defects.md) for additional guidance.
4047

4148
*From example.h:*
4249

@@ -48,8 +55,16 @@ void example_func(_Out_writes_z_(n) char* buffer, int n);
4855
4956
```cpp
5057
_Use_decl_annotations_
51-
void example_func(_Out_writes_z_(n) char* buffer, int n)
58+
void example_func(char* buffer, int n)
5259
{
5360
buffer[n] = '\0';
5461
}
5562
```
63+
64+
## See also
65+
66+
[Rule sets for C++ code](./using-rule-sets-to-specify-the-cpp-rules-to-run.md)\
67+
[Using SAL Annotations to reduce code defects](using-sal-annotations-to-reduce-c-cpp-code-defects.md)\
68+
[C28252](C28252.md)\
69+
[C28253](C28253.md)\
70+
[C28301](C28301.md)

docs/code-quality/c6029.md

Lines changed: 49 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4,76 +4,85 @@ title: Warning C6029
44
ms.date: 10/04/2022
55
f1_keywords: ["C6029", "USING_TAINTED_DATA", "__WARNING_USING_TAINTED_DATA"]
66
helpviewer_keywords: ["C6029"]
7-
ms.assetid: 07f89261-1b77-4597-9f34-12ce5d569b60
87
---
98
# Warning C6029
109

11-
> Possible buffer overrun in call to '*function*': use of unchecked value
10+
> Possible buffer overrun in call to '*function*'
1211
13-
## Remarks
12+
Possible buffer overrun in function called due to an unchecked buffer length/size parameter.
1413

15-
This warning indicates that a function that takes a buffer and a size is being passed an unchecked size. The data read-in from some external source hasn't been verified to see whether it's smaller than the buffer size. An attacker might intentionally specify a much larger than expected value for the size, which will lead to a buffer overrun.
14+
## Remarks
1615

17-
Generally, whenever you read data from an untrusted external source, make sure to verify it for validity. It's appropriate to verify the size to make sure it's in the expected range.
16+
This warning indicates that a function taking a buffer and a size is being passed an unchecked size. The data read-in from some external source hasn't been verified to see whether it's smaller than the buffer size. An attacker might intentionally specify a much larger than expected value for the size, which will lead to a buffer overrun. Generally, whenever you read data from an untrusted external source, make sure to verify it for validity. It's appropriate to verify the size to make sure it's in the expected range.
1817

1918
Code analysis name: `USING_TAINTED_DATA`
2019

2120
## Example
2221

23-
The following code generates this warning by calling the annotated function [`ReadFile`](/windows/desktop/api/fileapi/nf-fileapi-readfile) two times. After the first call, the Post attribute property marks the second parameter value untrusted. Therefore, passing an untrusted value in the second call to `ReadFile` generates this warning as shown in the following code:
22+
The following code generates this warning by calling the annotated function `std::fread` two times. The first call is used to determine the length of the data to read in later calls. After the first call `dataSize` has been marked by the analysis as coming from an untrusted source. Therefore, passing the untrusted value to the second `std::fread` call generates this warning. A malicious actor would be able to modify the file and cause the call to `std::fread` to overflow the `buffer` array. In read world code there should also be error recovery based on the return value of `std::fread`, this is purposely left out of these examples for simplicity.
2423

2524
```cpp
26-
#include "windows.h"
25+
void processData(FILE* file)
26+
{
27+
const size_t MAX_BUFFER_SIZE = 100;
28+
uint32_t buffer[MAX_BUFFER_SIZE]{};
29+
uint8_t dataSize = 0;
30+
31+
// Read length data from the beginning of the file
32+
fread(&dataSize, sizeof(uint8_t), 1, file);
33+
// Read the rest of the data based on the dataSize
34+
fread(buffer, sizeof(uint32_t), dataSize, file);
35+
}
36+
```
2737
28-
bool f(HANDLE hFile)
38+
The fix for the issue depends on the nature of the data and the behavior of the annotated function that is triggering the diagnostic. You may need to consult the documentation for that function for more information. A straightforward fix is to check the size before the second call to `std:fread`. In the next example we throw an exception to terminate the function, although most code would instead have an error recovery strategy that is specific to the scenario.
39+
40+
```cpp
41+
void processData(FILE* file)
2942
{
30-
char buff[MAX_PATH];
43+
const size_t MAX_BUFFER_SIZE = 100;
44+
uint32_t buffer[MAX_BUFFER_SIZE]{};
45+
uint8_t dataSize = 0;
3146
32-
DWORD cbLen;
33-
DWORD cbRead;
47+
fread(&dataSize, sizeof(uint32_t), 1, file);
3448
35-
// Read the number of byte to read (cbLen).
36-
if (!ReadFile (hFile, &cbLen, sizeof (cbLen), &cbRead, NULL))
37-
{
38-
return false;
39-
}
40-
// Read the bytes
41-
if (!ReadFile (hFile, buff, cbLen, &cbRead, NULL)) // warning C6029
49+
if( dataSize > MAX_BUFFER_SIZE)
4250
{
43-
return false;
51+
throw std::runtime_error("file data unexpected size");
4452
}
4553
46-
return true;
54+
fread(buffer, sizeof(uint32_t), dataSize, file);
4755
}
4856
```
4957

50-
To correct this warning, check the buffer size as shown in the following code:
58+
In the case of `std:fread` and similar functions, the data may need to be read even if it is large. This can be done by allocating the size of the buffer dynamically after the size becomes known or by calling `std:fread` multiple times as needed to read in the rest of the data. If allocating the buffer dynamically consider limiting the size to avoid introducing an out of memory exploit for large values, in our example this is not needed because it is already bounded by the size of `uint8_t`.
5159

5260
```cpp
53-
bool f(HANDLE hFile)
61+
void processDataDynamic(FILE* file)
5462
{
55-
char buff[MAX_PATH];
63+
uint8_t dataSize = 0;
64+
fread(&dataSize, sizeof(uint8_t), 1, file);
65+
66+
// Vector with `dataSize` default initialized objects
67+
std::vector<uint32_t> vecBuffer(dataSize);
5668

57-
DWORD cbLen;
58-
DWORD cbRead;
69+
fread(&vecBuffer[0], sizeof(uint32_t), dataSize, file);
70+
}
71+
void processDataMultiple(FILE* file)
72+
{
73+
const size_t MAX_BUFFER_SIZE = 100;
74+
uint32_t buffer[MAX_BUFFER_SIZE]{};
75+
uint8_t dataSize = 0;
5976

60-
// Read the number of byte to read (cbLen).
61-
if (!ReadFile (hFile, &cbLen, sizeof (cbLen), &cbRead, NULL))
62-
{
63-
return false;
64-
}
65-
// Ensure that there's enough space in the buffer to read that many bytes.
66-
if (cbLen > sizeof(buff))
67-
{
68-
return false;
69-
}
70-
// Read the bytes
71-
if (!ReadFile (hFile, buff, cbLen, &cbRead, NULL)) // warning C6029
77+
fread(&dataSize, sizeof(uint32_t), 1, file);
78+
79+
while( dataSize > 0 )
7280
{
73-
return false;
81+
size_t readSize = dataSize > MAX_BUFFER_SIZE ? MAX_BUFFER_SIZE : dataSize;
82+
fread(buffer, sizeof(uint32_t), readSize, file);
83+
dataSize = dataSize - readSize;
84+
// Process the data in `buffer`...
7485
}
75-
76-
return true;
7786
}
7887
```
7988

docs/code-quality/c6101.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,4 @@ SuccessWhenTrue InitNotZero(_Out_ int* output, int input)
8282
## See also
8383
8484
[Rule sets for C++ code](./using-rule-sets-to-specify-the-cpp-rules-to-run.md)\
85-
[Using SAL Annotations to Reduce C/C++ Code Defects](./using-sal-annotations-to-reduce-c-cpp-code-defects.md)
85+
[Using SAL Annotations to Reduce C/C++ Code Defects](./using-sal-annotations-to-reduce-c-cpp-code-defects.md)

0 commit comments

Comments
 (0)