hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-29 02:35:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

CPP: Update the ql, qhelp and example.

Browse Source
This commit is contained in:
Geoffrey White
2019-05-10 09:16:27 +01:00
parent 1f80dea375
commit 88f363d564
7 changed files with 27 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
cpp/ql/src/Security/CWE/CWE-676/DangerousUseOfGets.c
View File

@@ -1,14 +1,15 @@
// BAD: using gmtime
int is_morning_bad() {
const time_t now_seconds = time(NULL);
struct tm *now = gmtime(&now_seconds);
return (now->tm_hour < 12);
#define BUFFERSIZE (1024)
// BAD: using gets
void echo_bad() {
char buffer[BUFFERSIZE];
gets(buffer);
printf("Input was: '%s'\n", buffer);
}
// GOOD: using gmtime_r
int is_morning_good() {
const time_t now_seconds = time(NULL);
struct tm now;
gmtime_r(&now_seconds, &now);
return (now.tm_hour < 12);
// GOOD: using fgets
void echo_good() {
char buffer[BUFFERSIZE];
fgets(buffer, BUFFERSIZE, stdin);
printf("Input was: '%s'\n", buffer);
}

40
cpp/ql/src/Security/CWE/CWE-676/DangerousUseOfGets.qhelp
View File

@@ -3,49 +3,26 @@
"qhelp.dtd">
<qhelp>
<overview>
<p>This rule finds calls to functions that are dangerous to
use. Currently, it checks for calls
to <code>gets</code>, <code>gmtime</code>, <code>localtime</code>,
<code>ctime</code> and <code>asctime</code>. See <strong>Related
<p>This rule finds calls to the <code>gets</code> function, which is dangerous and
should not be used. See <strong>Related
rules</strong> below for rules that identify other dangerous functions.</p>
<p>The <code>gets</code> function is one of the vulnerabilities exploited by the Internet Worm of 1988, one of the first computer worms to spread through the Internet. The <code>gets</code> function provides no way to limit the amount of data that is read and stored, so without prior knowledge of the input it is impossible to use it safely with any size of buffer.</p>
<p>The time related functions such as <code>gmtime</code>
fill data into a <code>tm</code> struct or <code>char</code> array in
shared memory and then returns a pointer to that memory. If
the function is called from multiple places in the same program, and
especially if it is called from multiple threads in the same program,
then the calls will overwrite each other's data.</p>
</overview>
<recommendation>
<p>Replace calls to <code>gets</code> with <code>fgets</code>, specifying the maximum length to copy. This will prevent the buffer overflow.</p>
<p>Replace calls to <code>gmtime</code> with <code>gmtime_r</code>.
With <code>gmtime_r</code>, the application code manages allocation of
the <code>tm</code> struct. That way, separate calls to the function
can use their own storage.</p>
<p>Similarly replace calls to <code>localtime</code> with
<code>localtime_r</code>, calls to <code>ctime</code> with
<code>ctime_r</code> and calls to <code>asctime</code> with
<code>asctime_r</code>.</p>
</recommendation>
<example>
<p>The following example checks the local time in two ways:</p>
<sample src="PotentiallyDangerousFunction.c" />
<p>The first version uses <code>gmtime</code>, so it is vulnerable to
its data being overwritten by another thread. Even if this code is not
used in a multi-threaded context right now, future changes may
make the program multi-threaded. The second version of the code
uses <code>gmtime_r</code>. Since it allocates a new <code>tm</code>
struct on every call, it is immune to other calls to <code>gmtime</code>
or <code>gmtime_r</code>.</p>
<p>The following example gets a string from standard input in two ways:</p>
<sample src="DangerousUseOfGets" />
<p>The first version uses <code>gets</code> and will overflow if the input
is longer than the buffer. The second version of the code
uses <code>fgets</code> and will not overflow, because the amount of data
written is limited by the length parameter.</p>
</example>
<section title="Related rules">
<p>Other dangerous functions identified by CWE-676 ("Use of
@@ -61,6 +38,5 @@ rules for the following CWEs:</p>
<references>
<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Morris_worm">Morris worm</a>.</li>
<li>E. Spafford. <i>The Internet Worm Program: An Analysis</i>. Purdue Technical Report CSD-TR-823, <a href="http://www.textfiles.com/100/tr823.txt">(online)</a>, 1988.</li>
<li>SEI CERT C Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/CON33-C.+Avoid+race+conditions+when+using+library+functions">CON33-C. Avoid race conditions when using library functions</a>.</li>
</references>
</qhelp>

26
cpp/ql/src/Security/CWE/CWE-676/DangerousUseOfGets.ql
View File

@@ -1,6 +1,6 @@
/**
* @name Use of potentially dangerous function
* @description Certain standard library functions are dangerous to call.
* @name Use of dangerous function 'gets'
* @description The standard library 'gets' function is dangerous and should not be used.
* @kind problem
* @problem.severity error
* @precision high
@@ -11,24 +11,8 @@
*/
import cpp
predicate potentiallyDangerousFunction(Function f, string message) {
exists(string name | name = f.getQualifiedName() |
(
name = "gmtime" or
name = "localtime" or
name = "ctime" or
name = "asctime"
) and
message = "Call to " + name + " is potentially dangerous"
) or (
f.getQualifiedName() = "gets" and
message = "gets does not guard against buffer overflow"
)
}
from FunctionCall call, Function target, string message
from FunctionCall call, Function target
where
call.getTarget() = target and
potentiallyDangerousFunction(target, message)
select call, message
target.getQualifiedName() = "gets"
select call, "gets does not guard against buffer overflow"

22
cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.qhelp
View File

@@ -5,11 +5,8 @@
<overview>
<p>This rule finds calls to functions that are dangerous to
use. Currently, it checks for calls
to <code>gets</code>, <code>gmtime</code>, <code>localtime</code>,
<code>ctime</code> and <code>asctime</code>. See <strong>Related
rules</strong> below for rules that identify other dangerous functions.</p>
<p>The <code>gets</code> function is one of the vulnerabilities exploited by the Internet Worm of 1988, one of the first computer worms to spread through the Internet. The <code>gets</code> function provides no way to limit the amount of data that is read and stored, so without prior knowledge of the input it is impossible to use it safely with any size of buffer.</p>
to <code>gmtime</code>, <code>localtime</code>,
<code>ctime</code> and <code>asctime</code>.
<p>The time related functions such as <code>gmtime</code>
fill data into a <code>tm</code> struct or <code>char</code> array in
@@ -21,8 +18,6 @@ then the calls will overwrite each other's data.</p>
</overview>
<recommendation>
<p>Replace calls to <code>gets</code> with <code>fgets</code>, specifying the maximum length to copy. This will prevent the buffer overflow.</p>
<p>Replace calls to <code>gmtime</code> with <code>gmtime_r</code>.
With <code>gmtime_r</code>, the application code manages allocation of
the <code>tm</code> struct. That way, separate calls to the function
@@ -47,20 +42,7 @@ struct on every call, it is immune to other calls to <code>gmtime</code>
or <code>gmtime_r</code>.</p>
</example>
<section title="Related rules">
<p>Other dangerous functions identified by CWE-676 ("Use of
Potentially Dangerous Function") include <code>strcpy</code>
and <code>strcat</code>. Use of these functions is highlighted by
rules for the following CWEs:</p>
<ul>
<li>CWE-120 Classic Buffer Overflow
</li><li>CWE-131 Incorrect Calculation of Buffer Size
</li></ul>
</section>
<references>
<li>Wikipedia: <a href="http://en.wikipedia.org/wiki/Morris_worm">Morris worm</a>.</li>
<li>E. Spafford. <i>The Internet Worm Program: An Analysis</i>. Purdue Technical Report CSD-TR-823, <a href="http://www.textfiles.com/100/tr823.txt">(online)</a>, 1988.</li>
<li>SEI CERT C Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/CON33-C.+Avoid+race+conditions+when+using+library+functions">CON33-C. Avoid race conditions when using library functions</a>.</li>
</references>
</qhelp>

3
cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql
View File

@@ -20,9 +20,6 @@ predicate potentiallyDangerousFunction(Function f, string message) {
name = "asctime"
) and
message = "Call to " + name + " is potentially dangerous"
) or (
f.hasGlobalName("gets") and
message = "gets does not guard against buffer overflow"
)
}