Skip to content

Commit 9d49ad9

Browse files
redsun82redsun82
authored
C++: use includes in OverrunWrite qhelp files
Also added the relevant CERT C _and_ C++ standard references where they were missing, and did some minor stylistic tweaks to `OverrunWriteFloat.qhelp`.
1 parent c117a1e commit 9d49ad9

File tree

7 files changed

+37
-58
lines changed

7 files changed

+37
-58
lines changed

‎cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.qhelp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,5 @@
1818
<p>To fix the problem, either the second argument to <code>snprintf</code> should be changed to 80, or the buffer extended to 256 characters. A further improvement is to use a preprocessor define so that the size is only specified in one place, potentially preventing future recurrence of this issue.</p>
1919

2020
</example>
21-
<references>
22-
23-
24-
</references>
21+
<include src="OverrunWriteReferences.inc.qhelp" />
2522
</qhelp>

‎cpp/ql/src/Security/CWE/CWE-120/OverrunWrite.qhelp

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@
66
<p>The program performs a buffer copy or write operation with no upper limit on the size of the copy, and it appears that certain inputs will cause a buffer overflow to occur in this case. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
77

88
</overview>
9-
<recommendation>
10-
<p>Always control the length of buffer copy and buffer write operations. <code>strncpy</code> should be used over <code>strcpy</code>, <code>snprintf</code> over <code>sprintf</code>, and in other cases 'n-variant' functions should be preferred.</p>
11-
12-
</recommendation>
9+
<include src="OverrunWriteRecommendation.inc.qhelp" />
1310
<example>
1411
<sample src="OverrunWrite.c" />
1512

@@ -24,13 +21,5 @@
2421
</ul>
2522

2623
</example>
27-
<references>
28-
29-
<li>CERT C Coding Standard: <a href="https://www.securecoding.cert.org/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.</li>
30-
31-
32-
<!-- LocalWords: preprocessor CWE STR
33-
-->
34-
35-
</references>
24+
<include src="OverrunWriteReferences.inc.qhelp" />
3625
</qhelp>

‎cpp/ql/src/Security/CWE/CWE-120/OverrunWriteFloat.qhelp

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,30 +6,19 @@
66
<p>The program performs a buffer copy or write operation that includes one or more float to string conversions (i.e. the %f format specifier), which may overflow the destination buffer if extreme inputs are given. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
77

88
</overview>
9-
<recommendation>
10-
<p>Always control the length of buffer copy and buffer write operations. <code>strncpy</code> should be used over <code>strcpy</code>, <code>snprintf</code> over <code>sprintf</code>, and in other cases 'n-variant' functions should be preferred.</p>
11-
12-
</recommendation>
9+
<include src="OverrunWriteRecommendation.inc.qhelp" />
1310
<example>
1411
<sample src="OverrunWriteFloat.c" />
1512

16-
<p>In this example, the call to <code>sprintf</code> contains a %f format specifier. Though a 256 character buffer has been allowed, it is not sufficient for the most extreme floating point inputs. For example the representation of double value 1e304 (that is 1 with 304 zeroes after it) will overflow a buffer of this length.</p>
13+
<p>In this example, the call to <code>sprintf</code> contains a <code>%f</code> format specifier. Though a 256 character buffer has been allowed, it is not sufficient for the most extreme floating point inputs. For example the representation of double value 1e304 (that is 1 with 304 zeroes after it) will overflow a buffer of this length.</p>
1714

1815
<p>To fix this issue three changes should be made:</p>
1916
<ul>
2017
<li>Control the size of the buffer using a preprocessor define.</li>
2118
<li>Replace the call to <code>sprintf</code> with <code>snprintf</code>, specifying the define as the maximum length to copy. This will prevent the buffer overflow.</li>
22-
<li>Consider using the %g format specifier instead of %f.</li>
19+
<li>Consider using the <code>%g</code> format specifier instead of <code>%f</code>.</li>
2320
</ul>
2421

2522
</example>
26-
<references>
27-
28-
<li>CERT C Coding
29-
Standard: <a href="https://www.securecoding.cert.org/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR31-C. Guarantee
30-
that storage for strings has sufficient space for character data and
31-
the null terminator</a>.</li>
32-
33-
34-
</references>
23+
<include src="OverrunWriteReferences.inc.qhelp" />
3524
</qhelp>

‎cpp/ql/src/Security/CWE/CWE-120/OverrunWriteRecommendation.inc.qhelp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<fragment>
6+
<recommendation>
7+
<p>Always control the length of buffer copy and buffer write operations. <code>strncpy</code> should be used over <code>strcpy</code>, <code>snprintf</code> over <code>sprintf</code>, and in other cases 'n-variant' functions should be preferred.</p>
8+
</recommendation>
9+
</fragment>
10+
</qhelp>

‎cpp/ql/src/Security/CWE/CWE-120/OverrunWriteReferences.inc.qhelp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<fragment>
6+
<references>
7+
8+
<li>CERT C Coding Standard: <a href="https://www.securecoding.cert.org/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.</li>
9+
<li>CERT C++ Coding Standard: <a href="https://www.securecoding.cert.org/confluence/display/cplusplus/STR50-CPP.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR50-CPP. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.</li>
10+
11+
<!-- LocalWords: preprocessor CWE STR
12+
-->
13+
14+
</references>
15+
</fragment>
16+
</qhelp>

‎cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.qhelp

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@
66
<p>The program performs a buffer copy or write operation with no upper limit on the size of the copy. An unexpectedly long input that reaches this code will cause the buffer to overflow. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
77

88
</overview>
9-
<recommendation>
10-
<p>Always control the length of buffer copy and buffer write operations. <code>strncpy</code> should be used over <code>strcpy</code>, <code>snprintf</code> over <code>sprintf</code> etc. In general 'n-variant' functions should be preferred.</p>
11-
12-
</recommendation>
9+
<include src="OverrunWriteRecommendation.inc.qhelp" />
1310
<example>
1411
<sample src="UnboundedWrite.c" />
1512

@@ -18,13 +15,5 @@
1815
<p>To fix the problem the call to <code>sprintf</code> should be replaced with <code>snprintf</code>, specifying a maximum length of 80 characters.</p>
1916

2017
</example>
21-
<references>
22-
23-
<li>CERT C++ Coding Standard: <a href="https://www.securecoding.cert.org/confluence/display/cplusplus/STR50-CPP.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR50-CPP. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.</li>
24-
25-
26-
<!-- LocalWords: CWE STR
27-
-->
28-
29-
</references>
18+
<include src="OverrunWriteReferences.inc.qhelp" />
3019
</qhelp>

‎cpp/ql/src/Security/CWE/CWE-120/VeryLikelyOverrunWrite.qhelp

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@
66
<p>The program performs a buffer copy or write operation with no upper limit on the size of the copy, and by analysing the bounds of the expressions involved it appears that certain inputs will cause a buffer overflow to occur in this case. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
77

88
</overview>
9-
<recommendation>
10-
<p>Always control the length of buffer copy and buffer write operations. <code>strncpy</code> should be used over <code>strcpy</code>, <code>snprintf</code> over <code>sprintf</code>, and in other cases 'n-variant' functions should be preferred.</p>
11-
12-
</recommendation>
9+
<include src="OverrunWriteRecommendation.inc.qhelp" />
1310
<example>
1411
<sample src="VeryLikelyOverrunWrite.c" />
1512

@@ -23,13 +20,5 @@
2320
</ul>
2421

2522
</example>
26-
<references>
27-
28-
<li>CERT C Coding Standard: <a href="https://www.securecoding.cert.org/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.</li>
29-
30-
31-
<!-- LocalWords: preprocessor CWE STR
32-
-->
33-
34-
</references>
23+
<include src="OverrunWriteReferences.inc.qhelp" />
3524
</qhelp>

0 commit comments

Comments
 (0)