Skip to content

Commit 1fc8306

Browse files
knewbury01knewbury01
committed
Declarations7: add missing help file DCL39-C
1 parent b7b7ea7 commit 1fc8306

File tree

1 file changed

+277
-2
lines changed

1 file changed

+277
-2
lines changed

‎c/cert/src/rules/DCL39-C/InformationLeakageAcrossTrustBoundariesC.md

Lines changed: 277 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,284 @@ This query implements the CERT-C rule DCL39-C:
55
> Avoid information leakage when passing a structure across a trust boundary
66
77

8-
## CERT
98

10-
** REPLACE THIS BY RUNNING THE SCRIPT `scripts/help/cert-help-extraction.py` **
9+
## Description
10+
11+
The C Standard, 6.7.2.1, discusses the layout of structure fields. It specifies that non-bit-field members are aligned in an [implementation-defined](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-implementation-definedbehavior) manner and that there may be padding within or at the end of a structure. Furthermore, initializing the members of the structure does not guarantee initialization of the padding bytes. The C Standard, 6.2.6.1, paragraph 6 \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO-IEC9899-2011)\], states
12+
13+
> When a value is stored in an object of structure or union type, including in a member object, the bytes of the object representation that correspond to any padding bytes take unspecified values.
14+
15+
16+
Additionally, the storage units in which a bit-field resides may also have padding bits. For an object with automatic storage duration, these padding bits do not take on specific values and can contribute to leaking sensitive information.
17+
18+
When passing a pointer to a structure across a trust boundary to a different trusted domain, the programmer must ensure that the padding bytes and bit-field storage unit padding bits of such a structure do not contain sensitive information.
19+
20+
## Noncompliant Code Example
21+
22+
This noncompliant code example runs in kernel space and copies data from `arg` to user space. However, padding bytes may be used within the structure, for example, to ensure the proper alignment of the structure members. These padding bytes may contain sensitive information, which may then be leaked when the data is copied to user space.
23+
24+
```cpp
25+
#include <stddef.h>
26+
27+
struct test {
28+
int a;
29+
char b;
30+
int c;
31+
};
32+
33+
/* Safely copy bytes to user space */
34+
extern int copy_to_user(void *dest, void *src, size_t size);
35+
36+
void do_stuff(void *usr_buf) {
37+
struct test arg = {.a = 1, .b = 2, .c = 3};
38+
copy_to_user(usr_buf, &arg, sizeof(arg));
39+
}
40+
41+
```
42+
43+
## Noncompliant Code Example (memset())
44+
45+
The padding bytes can be explicitly initialized by calling `memset()`:
46+
47+
```cpp
48+
#include <string.h>
49+
50+
struct test {
51+
int a;
52+
char b;
53+
int c;
54+
};
55+
56+
/* Safely copy bytes to user space */
57+
extern int copy_to_user(void *dest, void *src, size_t size);
58+
59+
void do_stuff(void *usr_buf) {
60+
struct test arg;
61+
62+
/* Set all bytes (including padding bytes) to zero */
63+
memset(&arg, 0, sizeof(arg));
64+
65+
arg.a = 1;
66+
arg.b = 2;
67+
arg.c = 3;
68+
69+
copy_to_user(usr_buf, &arg, sizeof(arg));
70+
}
71+
72+
```
73+
However, a conforming compiler is free to implement `arg.b = 2` by setting the low-order bits of a register to 2, leaving the high-order bits unchanged and containing sensitive information. Then the platform copies all register bits into memory, leaving sensitive information in the padding bits. Consequently, this implementation could leak the high-order bits from the register to a user.
74+
75+
## Compliant Solution
76+
77+
This compliant solution serializes the structure data before copying it to an untrusted context:
78+
79+
```cpp
80+
#include <stddef.h>
81+
#include <string.h>
82+
83+
struct test {
84+
int a;
85+
char b;
86+
int c;
87+
};
88+
89+
/* Safely copy bytes to user space */
90+
extern int copy_to_user(void *dest, void *src, size_t size);
91+
92+
void do_stuff(void *usr_buf) {
93+
struct test arg = {.a = 1, .b = 2, .c = 3};
94+
/* May be larger than strictly needed */
95+
unsigned char buf[sizeof(arg)];
96+
size_t offset = 0;
97+
98+
memcpy(buf + offset, &arg.a, sizeof(arg.a));
99+
offset += sizeof(arg.a);
100+
memcpy(buf + offset, &arg.b, sizeof(arg.b));
101+
offset += sizeof(arg.b);
102+
memcpy(buf + offset, &arg.c, sizeof(arg.c));
103+
offset += sizeof(arg.c);
104+
/* Set all remaining bytes to zero */
105+
memset(buf + offset, 0, sizeof(arg) - offset);
106+
107+
copy_to_user(usr_buf, buf, offset /* size of info copied */);
108+
}
109+
```
110+
This code ensures that no uninitialized padding bytes are copied to unprivileged users. **Important:** The structure copied to user space is now a packed structure and the `copy_to_user()` function (or other eventual user) would need to unpack it to recreate the original padded structure.
111+
112+
## Compliant Solution (Padding Bytes)
113+
114+
Padding bytes can be explicitly declared as fields within the structure. This solution is not portable, however, because it depends on the [implementation](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-implementation) and target memory architecture. The following solution is specific to the x86-32 architecture:
115+
116+
```cpp
117+
#include <assert.h>
118+
#include <stddef.h>
119+
120+
struct test {
121+
int a;
122+
char b;
123+
char padding_1, padding_2, padding_3;
124+
int c;
125+
};
126+
127+
/* Safely copy bytes to user space */
128+
extern int copy_to_user(void *dest, void *src, size_t size);
129+
130+
void do_stuff(void *usr_buf) {
131+
/* Ensure c is the next byte after the last padding byte */
132+
static_assert(offsetof(struct test, c) ==
133+
offsetof(struct test, padding_3) + 1,
134+
"Structure contains intermediate padding");
135+
/* Ensure there is no trailing padding */
136+
static_assert(sizeof(struct test) ==
137+
offsetof(struct test, c) + sizeof(int),
138+
"Structure contains trailing padding");
139+
struct test arg = {.a = 1, .b = 2, .c = 3};
140+
arg.padding_1 = 0;
141+
arg.padding_2 = 0;
142+
arg.padding_3 = 0;
143+
copy_to_user(usr_buf, &arg, sizeof(arg));
144+
}
145+
146+
```
147+
The C Standard `static_assert()` macro accepts a constant expression and an [error message](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-error). The expression is evaluated at compile time and, if false, the compilation is terminated and the error message is output. (See [DCL03-C. Use a static assertion to test the value of a constant expression](https://wiki.sei.cmu.edu/confluence/display/c/DCL03-C.+Use+a+static+assertion+to+test+the+value+of+a+constant+expression) for more details.) The explicit insertion of the padding bytes into the `struct` should ensure that no additional padding bytes are added by the compiler and consequently both static assertions should be true. However, it is necessary to validate these assumptions to ensure that the solution is correct for a particular implementation.
148+
149+
## Compliant Solution (Structure Packing—GCC)
150+
151+
GCC allows specifying declaration attributes using the keyword `__attribute__((__packed__))`. When this attribute is present, the compiler will not add padding bytes for memory alignment unless an explicit alignment specifier for a structure member requires the introduction of padding bytes.
152+
153+
```cpp
154+
#include <stddef.h>
155+
156+
struct test {
157+
int a;
158+
char b;
159+
int c;
160+
} __attribute__((__packed__));
161+
162+
/* Safely copy bytes to user space */
163+
extern int copy_to_user(void *dest, void *src, size_t size);
164+
165+
void do_stuff(void *usr_buf) {
166+
struct test arg = {.a = 1, .b = 2, .c = 3};
167+
copy_to_user(usr_buf, &arg, sizeof(arg));
168+
}
169+
170+
```
171+
172+
## Compliant Solution (Structure Packing—Microsoft Visual Studio)
173+
174+
Microsoft Visual Studio supports `#pragma pack()` to suppress padding bytes \[[MSDN](http://msdn.microsoft.com/en-us/library/2e70t5y1(v=vs.110).aspx)\]. The compiler adds padding bytes for memory alignment, depending on the current packing mode, but still honors the alignment specified by `__declspec(align())`. In this compliant solution, the packing mode is set to 1 in an attempt to ensure all fields are given adjacent offsets:
175+
176+
```cpp
177+
#include <stddef.h>
178+
179+
#pragma pack(push, 1) /* 1 byte */
180+
struct test {
181+
int a;
182+
char b;
183+
int c;
184+
};
185+
#pragma pack(pop)
186+
187+
/* Safely copy bytes to user space */
188+
extern int copy_to_user(void *dest, void *src, size_t size);
189+
190+
void do_stuff(void *usr_buf) {
191+
struct test arg = {1, 2, 3};
192+
copy_to_user(usr_buf, &arg, sizeof(arg));
193+
}
194+
195+
```
196+
The `pack` pragma takes effect at the first `struct` declaration after the pragma is seen.
197+
198+
## Noncompliant Code Example
199+
200+
This noncompliant code example also runs in kernel space and copies data from `struct test` to user space. However, padding bits will be used within the structure due to the bit-field member lengths not adding up to the number of bits in an `unsigned` object. Further, there is an unnamed bit-field that causes no further bit-fields to be packed into the same storage unit. These padding bits may contain sensitive information, which may then be leaked when the data is copied to user space. For instance, the uninitialized bits may contain a sensitive kernel space pointer value that can be trivially reconstructed by an attacker in user space.
201+
202+
```cpp
203+
#include <stddef.h>
204+
205+
struct test {
206+
unsigned a : 1;
207+
unsigned : 0;
208+
unsigned b : 4;
209+
};
210+
211+
/* Safely copy bytes to user space */
212+
extern int copy_to_user(void *dest, void *src, size_t size);
213+
214+
void do_stuff(void *usr_buf) {
215+
struct test arg = { .a = 1, .b = 10 };
216+
copy_to_user(usr_buf, &arg, sizeof(arg));
217+
}
218+
```
219+
220+
## Compliant Solution
221+
222+
Padding bits can be explicitly declared, allowing the programmer to specify the value of those bits. When explicitly declaring all of the padding bits, any unnamed bit-fields of length `0` must be removed from the structure because the explicit padding bits ensure that no further bit-fields will be packed into the same storage unit.
223+
224+
```cpp
225+
#include <assert.h>
226+
#include <limits.h>
227+
#include <stddef.h>
228+
229+
struct test {
230+
unsigned a : 1;
231+
unsigned padding1 : sizeof(unsigned) * CHAR_BIT - 1;
232+
unsigned b : 4;
233+
unsigned padding2 : sizeof(unsigned) * CHAR_BIT - 4;
234+
};
235+
/* Ensure that we have added the correct number of padding bits. */
236+
static_assert(sizeof(struct test) == sizeof(unsigned) * 2,
237+
"Incorrect number of padding bits for type: unsigned");
238+
239+
/* Safely copy bytes to user space */
240+
extern int copy_to_user(void *dest, void *src, size_t size);
241+
242+
void do_stuff(void *usr_buf) {
243+
struct test arg = { .a = 1, .padding1 = 0, .b = 10, .padding2 = 0 };
244+
copy_to_user(usr_buf, &arg, sizeof(arg));
245+
}
246+
```
247+
This solution is not portable, however, because it depends on the [implementation](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-implementation) and target memory architecture. The explicit insertion of padding bits into the `struct` should ensure that no additional padding bits are added by the compiler. However, it is still necessary to validate these assumptions to ensure that the solution is correct for a particular implementation. For instance, the DEC Alpha is an example of a 64-bit architecture with 32-bit integers that allocates 64 bits to a storage unit.
248+
249+
In addition, this solution assumes that there are no integer padding bits in an `unsigned int`. The portable version of the width calculation from [INT35-C. Use correct integer precisions](https://wiki.sei.cmu.edu/confluence/display/c/INT35-C.+Use+correct+integer+precisions) cannot be used because the bit-field width must be an integer constant expression.
250+
251+
From this situation, it can be seen that special care must be taken because no solution to the bit-field padding issue will be 100% portable.
252+
253+
Risk Assessment
254+
255+
Padding units might contain sensitive data because the C Standard allows any padding to take [unspecified values](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-unspecifiedvalue). A pointer to such a structure could be passed to other functions, causing information leakage.
256+
257+
<table> <tbody> <tr> <th> Rule </th> <th> Severity </th> <th> Likelihood </th> <th> Remediation Cost </th> <th> Priority </th> <th> Level </th> </tr> <tr> <td> DCL39-C </td> <td> Low </td> <td> Unlikely </td> <td> High </td> <td> <strong>P1</strong> </td> <td> <strong>L3</strong> </td> </tr> </tbody> </table>
258+
259+
260+
## Automated Detection
261+
262+
<table> <tbody> <tr> <th> Tool </th> <th> Version </th> <th> Checker </th> <th> Description </th> </tr> <tr> <td> <a> Astrée </a> </td> <td> 22.04 </td> <td> <strong>function-argument-with-padding</strong> </td> <td> Partially checked </td> </tr> <tr> <td> <a> Axivion Bauhaus Suite </a> </td> <td> 7.2.0 </td> <td> <strong>CertC-DCL39</strong> </td> <td> Detects composite structures with padding, in particular those passed to trust boundary routines. </td> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.2p0 </td> <td> <strong>MISC.PADDING.POTB</strong> </td> <td> Padding Passed Across a Trust Boundary </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.4 </td> <td> <strong>DF4941, DF4942, DF4943</strong> </td> <td> </td> </tr> <tr> <td> <a> Klocwork </a> </td> <td> 2022.4 </td> <td> <strong>PORTING.STORAGE.STRUCT</strong> </td> <td> </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.2 </td> <td> <strong>CERT_C-DCL39-a</strong> </td> <td> A pointer to a structure should not be passed to a function that can copy data to the user space </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2022b </td> <td> <a> CERT C: Rule DCL39-C </a> </td> <td> Checks for information leak via structure padding </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>4941, 4942, 4943</strong> </td> <td> </td> </tr> <tr> <td> <a> PRQA QA-C++ </a> </td> <td> 4.4 </td> <td> <strong>4941, 4942, 4943</strong> </td> <td> </td> </tr> <tr> <td> <a> RuleChecker </a> </td> <td> 22.04 </td> <td> <strong>function-argument-with-padding</strong> </td> <td> Partially checked </td> </tr> </tbody> </table>
263+
264+
265+
## Related Vulnerabilities
266+
267+
Numerous vulnerabilities in the Linux Kernel have resulted from violations of this rule. [CVE-2010-4083](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2010-4083) describes a [vulnerability](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability) in which the `semctl()` system call allows unprivileged users to read uninitialized kernel stack memory because various fields of a `semid_ds struct` declared on the stack are not altered or zeroed before being copied back to the user.
268+
269+
[CVE-2010-3881](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2010-3881) describes a vulnerability in which structure padding and reserved fields in certain data structures in `QEMU-KVM` were not initialized properly before being copied to user space. A privileged host user with access to `/dev/kvm` could use this [flaw](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-securityflaw) to leak kernel stack memory to user space.
270+
271+
[CVE-2010-3477](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2010-3477) describes a kernel information leak in `act_police` where incorrectly initialized structures in the traffic-control dump code may allow the disclosure of kernel memory to user space applications.
272+
273+
Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability) resulting from the violation of this rule on the [CERT website](https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+DCL39-C).
274+
275+
## Related Guidelines
276+
277+
[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)
278+
279+
<table> <tbody> <tr> <th> Taxonomy </th> <th> Taxonomy item </th> <th> Relationship </th> </tr> <tr> <td> <a> CERT C Secure Coding Standard </a> </td> <td> <a> DCL03-C. Use a static assertion to test the value of a constant expression </a> </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> </tbody> </table>
280+
281+
282+
## Bibliography
283+
284+
<table> <tbody> <tr> <td> \[ <a> ISO/IEC 9899:2011 </a> \] </td> <td> 6.2.6.1, "General" 6.7.2.1, "Structure and Union Specifiers" </td> </tr> <tr> <td> \[ <a> Graff 2003 </a> \] </td> <td> </td> </tr> <tr> <td> \[ <a> Sun 1993 </a> \] </td> <td> </td> </tr> </tbody> </table>
285+
11286

12287
## Implementation notes
13288

0 commit comments

Comments
 (0)