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

auto format script

Browse Source
This commit is contained in:
james
2020-10-05 16:44:48 +01:00
parent 6667b58b2c
commit 47483a8e84
286 changed files with 98 additions and 16179 deletions
Download Patch File Download Diff File Expand all files Collapse all files

145
docs/language/query-help-markdown.py
View File

@@ -6,10 +6,11 @@ import sys
import os
# Define which languages and query packs to consider
languages = [ "cpp", "csharp", "go", "java", "javascript", "python"]
languages = ["cpp", "csharp", "go", "java", "javascript", "python"]
# Running generate query-help with "security-and-quality.qls" generates errors, so just use these two suites for now
packs = [ "code-scanning", "security-extended" ]
packs = ["code-scanning", "security-extended"]
def prefix_repo_nwo(filename):
"""
@@ -19,32 +20,37 @@ def prefix_repo_nwo(filename):
/home/alice/git/ql/java/ql/src/MyQuery.ql
becomes:
github/codeql/java/ql/src/MyQuery.ql
If we can't detect a known NWO (e.g. github/codeql, github/codeql-go), the
path will be truncated to the root of the git repo:
ql/java/ql/src/MyQuery.ql
If the filename is not part of a Git repo, the return value is the
same as the input value: the whole path.
"""
dirname = os.path.dirname(filename)
try:
git_toplevel_dir_subp = subprocess_run(["git", "-C", dirname, "rev-parse", "--show-toplevel"])
git_toplevel_dir_subp = subprocess_run(
["git", "-C", dirname, "rev-parse", "--show-toplevel"])
except:
# Not a Git repo
return filename
git_toplevel_dir = git_toplevel_dir_subp.stdout.strip()
# Detect 'github/codeql' and 'github/codeql-go' repositories by checking the remote (it's a bit
# of a hack but will work in most cases, as long as the remotes have 'codeql' and 'codeql-go'
# in the URL
git_remotes = subprocess_run(["git","-C",dirname,"remote","-v"]).stdout.strip()
git_remotes = subprocess_run(
["git", "-C", dirname, "remote", "-v"]).stdout.strip()
if "codeql-go" in git_remotes: prefix = "github/codeql-go"
elif "codeql" in git_remotes: prefix = "github/codeql"
else: prefix = os.path.basename(git_toplevel_dir)
if "codeql-go" in git_remotes:
prefix = "github/codeql-go"
elif "codeql" in git_remotes:
prefix = "github/codeql"
else:
prefix = os.path.basename(git_toplevel_dir)
return os.path.join(prefix, filename[len(git_toplevel_dir)+1:])
@@ -59,9 +65,11 @@ def single_spaces(input):
def get_query_metadata(key, metadata, queryfile):
"""Returns query metadata or prints a warning to stderr if a particular piece of metadata is not available."""
if key in metadata: return single_spaces(metadata[key])
if key in metadata:
return single_spaces(metadata[key])
query_id = metadata['id'] if 'id' in metadata else 'unknown'
print("Warning: no '%s' metadata for query with ID '%s' (%s)" % (key, query_id, queryfile), file=sys.stderr)
print("Warning: no '%s' metadata for query with ID '%s' (%s)" %
(key, query_id, queryfile), file=sys.stderr)
return ""
@@ -69,14 +77,15 @@ def subprocess_run(cmd):
"""Runs a command through subprocess.run, with a few tweaks. Raises an Exception if exit code != 0."""
return subprocess.run(cmd, capture_output=True, text=True, env=os.environ.copy(), check=True)
try: # Check for `git` on path
subprocess_run(["git","--version"])
try: # Check for `git` on path
subprocess_run(["git", "--version"])
except Exception as e:
print("Error: couldn't invoke 'git'. Is it on the path? Aborting.", file=sys.stderr)
raise e
try: # Check for `codeql` on path
subprocess_run(["codeql","--version"])
try: # Check for `codeql` on path
subprocess_run(["codeql", "--version"])
except Exception as e:
print("Error: couldn't invoke CodeQL CLI 'codeql'. Is it on the path? Aborting.", file=sys.stderr)
raise e
@@ -87,12 +96,12 @@ except Exception as e:
#
# (and assumes the codeql-go repo is in a similar location)
#codeql_search_path = "./ql" or "./codeql-go" # will be extended further down
# codeql_search_path = "./ql" or "./codeql-go" # will be extended further down
# Extend CodeQL search path by detecting root of the current Git repo (if any). This means that you
# can run this script from any location within the CodeQL git repository.
try:
git_toplevel_dir = subprocess_run(["git","rev-parse","--show-toplevel"])
git_toplevel_dir = subprocess_run(["git", "rev-parse", "--show-toplevel"])
# Current working directory is in a Git repo. Add it to the search path, just in case it's the CodeQL repo
#git_toplevel_dir = git_toplevel_dir.stdout.strip()
@@ -103,90 +112,104 @@ except:
pass
# Iterate over all languages and packs, and resolve which queries are part of those packs
for lang in languages:
for lang in languages:
# Define empty dictionary to store @name:filename pairs to generate alphabetically sorted Sphinx toctree
index_file_dictionary = {}
index_file_dictionary = {}
for pack in packs:
# Get absolute paths to queries in this pack by using 'codeql resolve queries'
try:
queries_subp = subprocess_run(["codeql","resolve","queries","--search-path", codeql_search_path, "%s-%s.qls" % (lang, pack)])
queries_subp = subprocess_run(
["codeql", "resolve", "queries", "--search-path", codeql_search_path, "%s-%s.qls" % (lang, pack)])
except Exception as e:
# Resolving queries might go wrong if the github/codeql and github/codeql-go repositories are not
# on the search path.
print(
"Warning: couldn't find query pack '%s' for language '%s'. Do you have the right repositories in the right places (search path: '%s')?" % (pack, lang, codeql_search_path),
"Warning: couldn't find query pack '%s' for language '%s'. Do you have the right repositories in the right places (search path: '%s')?" % (
pack, lang, codeql_search_path),
file=sys.stderr
)
)
continue
# Define empty dictionary to store @name:filename pairs to generate alphabetically sorted Sphinx toctree later
index_file_dictionary = {}
index_file_dictionary = {}
# Investigate metadata for every query by using 'codeql resolve metadata'
# Investigate metadata for every query by using 'codeql resolve metadata'
for queryfile in queries_subp.stdout.strip().split("\n"):
query_metadata_json = subprocess_run(["codeql","resolve","metadata",queryfile]).stdout.strip()
query_metadata_json = subprocess_run(
["codeql", "resolve", "metadata", queryfile]).stdout.strip()
meta = json.loads(query_metadata_json)
# Turn an absolute path to a query file into an nwo-prefixed path (e.g. github/codeql/java/ql/src/....)
queryfile_nwo = prefix_repo_nwo(queryfile)
# Generate the query help for each query
query_help = subprocess_run(["codeql","generate","query-help","--format=markdown","--warnings=hide",queryfile]).stdout.strip()
# Generate the query help for each query
query_help = subprocess_run(
["codeql", "generate", "query-help", "--format=markdown", "--warnings=hide", queryfile]).stdout.strip()
# Pull out relevant query metadata properties that we want to display in the query help
query_name_meta = get_query_metadata('name', meta, queryfile)
query_description = get_query_metadata('description', meta, queryfile)
query_id = "ID: " + get_query_metadata('id', meta, queryfile) + "\n"
query_kind = "Kind: " + get_query_metadata('kind', meta, queryfile) + "\n"
query_severity = "Severity: " + get_query_metadata('problem.severity', meta, queryfile) + "\n"
query_precision = "Precision: " + get_query_metadata('precision', meta, queryfile) + "\n"
query_tags = "Tags: " + get_query_metadata('tags', meta, queryfile) + "\n"
query_description = get_query_metadata(
'description', meta, queryfile)
query_id = "ID: " + \
get_query_metadata('id', meta, queryfile) + "\n"
query_kind = "Kind: " + \
get_query_metadata('kind', meta, queryfile) + "\n"
query_severity = "Severity: " + \
get_query_metadata('problem.severity', meta, queryfile) + "\n"
query_precision = "Precision: " + \
get_query_metadata('precision', meta, queryfile) + "\n"
query_tags = "Tags: " + \
get_query_metadata('tags', meta, queryfile) + "\n"
# Build a link to the query source file for display in the query help
if "go" in prefix_repo_nwo(queryfile):
transform_link = prefix_repo_nwo(queryfile).replace("codeql-go", "codeql-go/tree/main").replace(" ", "%20").replace("\\","/")
else:
transform_link = prefix_repo_nwo(queryfile).replace("codeql", "codeql/tree/main").replace(" ", "%20").replace("\\","/")
query_link = "[Click to see the query in the CodeQL repository](https://github.com/" + transform_link + ")\n"
transform_link = prefix_repo_nwo(queryfile).replace(
"codeql-go", "codeql-go/tree/main").replace(" ", "%20").replace("\\", "/")
else:
transform_link = prefix_repo_nwo(queryfile).replace(
"codeql", "codeql/tree/main").replace(" ", "%20").replace("\\", "/")
query_link = "[Click to see the query in the CodeQL repository](https://github.com/" + \
transform_link + ")\n"
# Join metadata into a literal block and add query link below
meta_string = "\n"*2 + "```\n" + query_id + query_kind + query_severity + query_precision + query_tags + "\n" + "```\n" + query_link + "\n"
meta_string = "\n"*2 + "```\n" + query_id + query_kind + query_severity + \
query_precision + query_tags + "\n" + "```\n" + query_link + "\n"
# Insert metadata block into query help directly under title
full_help = query_help.replace("\n", meta_string, 1)
# Basename of query, used to make markdown output filepath
query_name = os.path.split(queryfile_nwo)[1][:-3]
# Populate index_file_dictionary with @name extracted from metadata and corresponding query filename
index_file_dictionary[query_name_meta] = lang + "/" + query_name
# Populate index_file_dictionary with @name extracted from metadata and corresponding query filename
index_file_dictionary[query_name_meta] = lang + "/" + query_name
# Make paths for output of the form: query-help-markdown/<lang>/<queryfile>.md
docs_dir = 'query-help'
md_dir_path = os.path.join(docs_dir, lang)
md_file_path = os.path.join(md_dir_path, query_name + ".md")
# Make directories for output paths they don't already exist
# Make directories for output paths they don't already exist
if not os.path.isdir(md_dir_path):
os.makedirs(md_dir_path)
# Generate query help at chosen path if output file doesn't already exist
# Generate query help at chosen path if output file doesn't already exist
if not os.path.exists(md_file_path):
file = open(md_file_path, "x")
file.write(full_help)
file.close()
# Sort index_file_dictionary alphabetically by @name key, and create column of filename values
# Sort index_file_dictionary alphabetically by @name key, and create column of filename values
sorted_index = dict(sorted(index_file_dictionary.items()))
sorted_index = ("\n" + " ").join(sorted_index.values())
# Add directives to make sorted_index a valid toctree for sphinx source files
toc_directive = ".. toctree::\n :titlesonly:\n\n "
toc_include = toc_directive + sorted_index
# Write toctree to rst
# Write toctree to rst
toc_file = os.path.join(docs_dir, "toc-" + lang + ".rst")
file = open(toc_file, "x")
file.write(toc_include)
file.close()
file.close()

8
docs/language/query-help/conf.py
View File

@@ -45,11 +45,11 @@ project = u'CodeQL query help'
# Add md parser to process query help markdown files
source_parsers = {
'.md': 'recommonmark.parser.CommonMarkParser',
}
#source_parsers = {
# '.md': 'recommonmark.parser.CommonMarkParser',
#}
source_suffix = ['.rst', '.md']
source_suffix = ['.rst']
# -- Project-specifc options for HTML output ----------------------------------------------

53
docs/language/query-help/cpp/AllocaInLoop.md
View File

@@ -1,53 +0,0 @@
# Call to alloca in a loop
```
ID: cpp/alloca-in-loop
Kind: problem
Severity: warning
Precision: high
Tags: reliability correctness security external/cwe/cwe-770
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Memory%20Management/AllocaInLoop.ql)
The `alloca` macro allocates memory by expanding the current stack frame. Invoking `alloca` within a loop may lead to a stack overflow because the memory is not released until the function returns.
## Recommendation
Consider invoking `alloca` once outside the loop, or using `malloc` or `new` to allocate memory on the heap if the allocation must be done inside the loop.
## Example
The variable `path` is allocated inside a loop with `alloca`. Consequently, storage for all copies of the path is present in the stack frame until the end of the function.
```cpp
char *dir_path;
char **dir_entries;
int count;
for (int i = 0; i < count; i++) {
char *path = (char*)alloca(strlen(dir_path) + strlen(dir_entry[i]) + 2);
// use path
}
```
In the revised example, `path` is allocated with `malloc` and freed at the end of the loop.
```cpp
char *dir_path;
char **dir_entries;
int count;
for (int i = 0; i < count; i++) {
char *path = (char*)malloc(strlen(dir_path) + strlen(dir_entry[i]) + 2);
// use path
free(path);
}
```
## References
* Linux Programmer's Manual: [ALLOCA(3)](http://man7.org/linux/man-pages/man3/alloca.3.html).
* Common Weakness Enumeration: [CWE-770](https://cwe.mitre.org/data/definitions/770.html).

50
docs/language/query-help/cpp/ArithmeticUncontrolled.md
View File

@@ -1,50 +0,0 @@
# Uncontrolled data in arithmetic expression
```
ID: cpp/uncontrolled-arithmetic
Kind: path-problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-190 external/cwe/cwe-191
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql)
Performing calculations on uncontrolled data can result in integer overflows unless the input is validated.
If the data is not under your control, and can take extremely large values, even arithmetic operations that would usually result in a small change in magnitude may result in overflows.
## Recommendation
Always guard against overflow in arithmetic operations on uncontrolled data by doing one of the following:
* Validate the data.
* Define a guard on the arithmetic expression, so that the operation is performed only if the result can be known to be less than, or equal to, the maximum value for the type, for example `INT_MAX`.
* Use a wider type, so that larger input values do not cause overflow.
## Example
In this example, a random integer is generated. Because the value is not controlled by the programmer, it could be extremely large. Performing arithmetic operations on this value could therefore cause an overflow. To avoid this happening, the example shows how to perform a check before performing an arithmetic operation.
```c
int main(int argc, char** argv) {
int i = rand();
// BAD: potential overflow
int j = i + 1000;
// ...
int n = rand();
int k;
// GOOD: use a guard to prevent overflow
if (n < INT_MAX-1000)
k = n + 1000;
else
k = INT_MAX;
}
```
## References
* Common Weakness Enumeration: [CWE-190](https://cwe.mitre.org/data/definitions/190.html).
* Common Weakness Enumeration: [CWE-191](https://cwe.mitre.org/data/definitions/191.html).

61
docs/language/query-help/cpp/AuthenticationBypass.md
View File

@@ -1,61 +0,0 @@
# Authentication bypass by spoofing
```
ID: cpp/user-controlled-bypass
Kind: path-problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-290
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql)
Code which relies on an IP address or domain name for authentication can be exploited by an attacker who spoofs their address.
## Recommendation
IP address verification can be a useful part of an authentication scheme, but it should not be the single factor required for authentication. Make sure that other authentication methods are also in place.
## Example
In this example (taken from [CWE-290: Authentication Bypass by Spoofing](http://cwe.mitre.org/data/definitions/290.html)), the client is authenticated by checking that its IP address is `127.0.0.1`. An attacker might be able to bypass this authentication by spoofing their IP address.
```cpp
#define BUFFER_SIZE (4 * 1024)
void receiveData()
{
int sock;
sockaddr_in addr, addr_from;
char buffer[BUFFER_SIZE];
int msg_size;
socklen_t addr_from_len;
// configure addr
memset(&addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
addr.sin_port = htons(1234);
addr.sin_addr.s_addr = INADDR_ANY;
// create and bind the socket
sock = socket(AF_INET, SOCK_DGRAM, 0);
bind(sock, (sockaddr *)&addr, sizeof(addr));
// receive message
addr_from_len = sizeof(addr_from);
msg_size = recvfrom(sock, buffer, BUFFER_SIZE, 0, (sockaddr *)&addr_from, &addr_from_len);
// BAD: the address is controllable by the user, so it
// could be spoofed to bypass the security check below.
if ((msg_size > 0) && (strcmp("127.0.0.1", inet_ntoa(addr_from.sin_addr)) == 0))
{
// ...
}
}
```
## References
* Common Weakness Enumeration: [CWE-290](https://cwe.mitre.org/data/definitions/290.html).

46
docs/language/query-help/cpp/BadAdditionOverflowCheck.md
View File

@@ -1,46 +0,0 @@
# Bad check for overflow of integer addition
>ID: cpp/bad-addition-overflow-check
>
>Kind: problem
>
>Severity: error
>
>Precision: very-high
>Tags: reliability correctness security external/cwe/cwe-190 external/cwe/cwe-192
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Arithmetic/BadAdditionOverflowCheck.ql)
Checking for overflow of integer addition needs to be done with care, because automatic type promotion can prevent the check from working as intended, with the same value (`true` or `false`) always being returned.
## Recommendation
Use an explicit cast to make sure that the result of the addition is not implicitly converted to a larger type.
## Example
```cpp
bool checkOverflow(unsigned short x, unsigned short y) {
// BAD: comparison is always false due to type promotion
return (x + y < x);
}
```
On a typical architecture where `short` is 16 bits and `int` is 32 bits, the operands of the addition are automatically promoted to `int`, so it cannot overflow and the result of the comparison is always false.
The code below implements the check correctly, by using an explicit cast to make sure that the result of the addition is `unsigned short` (which may overflow, in which case the comparison would evaluate to `true`).
```cpp
bool checkOverflow(unsigned short x, unsigned short y) {
return ((unsigned short)(x + y) < x); // GOOD: explicit cast
}
```
## References
* [Preserving Rules](http://c-faq.com/expr/preservingrules.html)
* [Understand integer conversion rules](https://www.securecoding.cert.org/confluence/plugins/servlet/mobile#content/view/20086942)
* Common Weakness Enumeration: [CWE-190](https://cwe.mitre.org/data/definitions/190.html).
* Common Weakness Enumeration: [CWE-192](https://cwe.mitre.org/data/definitions/192.html).

42
docs/language/query-help/cpp/BadlyBoundedWrite.md
View File

@@ -1,42 +0,0 @@
# Badly bounded write
```
ID: cpp/badly-bounded-write
Kind: problem
Severity: error
Precision: high
Tags: reliability security external/cwe/cwe-120 external/cwe/cwe-787 external/cwe/cwe-805
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql)
The program performs a buffer copy or write operation with an incorrect upper limit on the size of the copy. A sufficiently long input will overflow the target buffer. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.
## Recommendation
Use preprocessor defines to specify the size of buffers, and use the same defines as arguments to `strncpy`, `snprintf` etc. This technique will ensure that buffer sizes are always specified correctly so that no overflow occurs.
## Example
```c
void congratulateUser(const char *userName)
{
char buffer[80];
// BAD: even though snprintf is used, this could overflow the buffer
// because the size specified is too large.
snprintf(buffer, 256, "Congratulations, %s!", userName);
MessageBox(hWnd, buffer, "New Message", MB_OK);
}
```
In this example, the developer has used `snprintf` to control the maximum number of characters that can be written to `buffer`. Unfortunately, perhaps due to modifications since the code was first written, a limited buffer overrun can still occur because the size argument to `snprintf` is larger than the actual size of the buffer.
To fix the problem, either the second argument to `snprintf` 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.
## References
* Common Weakness Enumeration: [CWE-120](https://cwe.mitre.org/data/definitions/120.html).
* Common Weakness Enumeration: [CWE-787](https://cwe.mitre.org/data/definitions/787.html).
* Common Weakness Enumeration: [CWE-805](https://cwe.mitre.org/data/definitions/805.html).

46
docs/language/query-help/cpp/BrokenCryptoAlgorithm.md
View File

@@ -1,46 +0,0 @@
# Use of a broken or risky cryptographic algorithm
```
ID: cpp/weak-cryptographic-algorithm
Kind: problem
Severity: error
Precision: medium
Tags: security external/cwe/cwe-327
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql)
Using broken or weak cryptographic algorithms can leave data vulnerable to being decrypted.
Many cryptographic algorithms provided by cryptography libraries are known to be weak, or flawed. Using such an algorithm means that an attacker may be able to easily decrypt the encrypted data.
## Recommendation
Ensure that you use a strong, modern cryptographic algorithm. Use at least AES-128 or RSA-2048.
## Example
The following code shows an example of using the `advapi` windows API to decrypt some data. When creating a key, you must specify which algorithm to use. The first example uses DES which is an older algorithm that is now considered weak. The second example uses AES, which is a strong modern algorithm.
```c
void advapi() {
HCRYPTPROV hCryptProv;
HCRYPTKEY hKey;
HCRYPTHASH hHash;
// other preparation goes here
// BAD: use 3DES for key
CryptDeriveKey(hCryptProv, CALG_3DES, hHash, 0, &hKey);
// GOOD: use AES
CryptDeriveKey(hCryptProv, CALG_AES_256, hHash, 0, &hKey);
}
```
## References
* NIST, FIPS 140 Annex a: [ Approved Security Functions](http://csrc.nist.gov/publications/fips/fips140-2/fips1402annexa.pdf).
* NIST, SP 800-131A: [ Transitions: Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths](http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf).
* Common Weakness Enumeration: [CWE-327](https://cwe.mitre.org/data/definitions/327.html).

48
docs/language/query-help/cpp/CastArrayPointerArithmetic.md
View File

@@ -1,48 +0,0 @@
# Upcast array used in pointer arithmetic
```
ID: cpp/upcast-array-pointer-arithmetic
Kind: path-problem
Severity: warning
Precision: high
Tags: correctness reliability security external/cwe/cwe-119 external/cwe/cwe-843
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Conversion/CastArrayPointerArithmetic.ql)
A pointer to a derived class may be implicitly converted to a pointer to its base type when passed as an argument to a function expecting a pointer to the base type. If pointer arithmetic or an array dereference is then used, it will be performed using the size of the base type. This can lead to reading data from unexpected fields in the derived type.
## Recommendation
Only convert pointers to single objects. If you must work with a sequence of objects that are converted to a base type, use an array of pointers rather than a pointer to an array.
## Example
```cpp
class Base {
public:
int x;
}
class Derived: public Base {
public:
int y;
};
void dereference_base(Base *b) {
b[2].x;
}
void dereference_derived(Derived *d) {
d[2].x;
}
void test () {
Derived[4] d;
dereference_base(d); // BAD: implicit conversion to Base*
dereference_derived(d); // GOOD: implicit conversion to Derived*, which will be the right size
}
```

54
docs/language/query-help/cpp/CgiXss.md
View File

@@ -1,54 +0,0 @@
# CGI script vulnerable to cross-site scripting
```
ID: cpp/cgi-xss
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-079
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql)
Directly writing an HTTP request parameter back to a web page allows for a cross-site scripting vulnerability. The data is displayed in a user's web browser as belonging to one site, but it is provided by some other site that the user browses to. In effect, such an attack allows one web site to insert content in the other one.
For web servers implemented with the Common Gateway Interface (CGI), HTTP parameters are supplied via the `QUERY_STRING` environment variable.
## Recommendation
To guard against cross-site scripting, consider escaping special characters before writing the HTTP parameter back to the page.
## Example
In the following example, the `bad_server` writes a parameter directly back to the HTML page that the user will see. The `good_server` first escapes any HTML special characters before writing to the HTML page.
```c
void bad_server() {
char* query = getenv("QUERY_STRING");
puts("<p>Query results for ");
// BAD: Printing out an HTTP parameter with no escaping
puts(query);
puts("\n<p>\n");
puts(do_search(query));
}
void good_server() {
char* query = getenv("QUERY_STRING");
puts("<p>Query results for ");
// GOOD: Escape HTML characters before adding to a page
char* query_escaped = escape_html(query);
puts(query_escaped);
free(query_escaped);
puts("\n<p>\n");
puts(do_search(query));
}
```
## References
* OWASP: [XSS (Cross Site Scripting) Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html).
* Wikipedia: [Cross-site scripting](http://en.wikipedia.org/wiki/Cross-site_scripting).
* IETF Tools: [The Common Gateway Specification (CGI)](http://tools.ietf.org/html/draft-robinson-www-interface-00).
* Common Weakness Enumeration: [CWE-79](https://cwe.mitre.org/data/definitions/79.html).

45
docs/language/query-help/cpp/CleartextBufferWrite.md
View File

@@ -1,45 +0,0 @@
# Cleartext storage of sensitive information in buffer
```
ID: cpp/cleartext-storage-buffer
Kind: path-problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-312
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-311/CleartextBufferWrite.ql)
Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.
## Recommendation
Ensure that sensitive information is always encrypted before being stored, especially before writing to a file. It may be wise to encrypt information before it is put into a buffer that may be readable in memory.
In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.
## Example
The following example shows two ways of storing user credentials in a file. In the 'BAD' case, the credentials are simply stored in cleartext. In the 'GOOD' case, the credentials are encrypted before storing them.
```c
void writeCredentials() {
char *password = "cleartext password";
FILE* file = fopen("credentials.txt", "w");
// BAD: write password to disk in cleartext
fputs(password, file);
// GOOD: encrypt password first
char *encrypted = encrypt(password);
fputs(encrypted, file);
}
```
## References
* M. Dowd, J. McDonald and J. Schuhm, *The Art of Software Security Assessment*, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
* M. Howard and D. LeBlanc, *Writing Secure Code*, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
* Common Weakness Enumeration: [CWE-312](https://cwe.mitre.org/data/definitions/312.html).

45
docs/language/query-help/cpp/CleartextFileWrite.md
View File

@@ -1,45 +0,0 @@
# Cleartext storage of sensitive information in file
```
ID: cpp/cleartext-storage-file
Kind: problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-313
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.ql)
Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage.
## Recommendation
Ensure that sensitive information is always encrypted before being stored, especially before writing to a file. It may be wise to encrypt information before it is put into a buffer that may be readable in memory.
In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.
## Example
The following example shows two ways of storing user credentials in a file. In the 'BAD' case, the credentials are simply stored in cleartext. In the 'GOOD' case, the credentials are encrypted before storing them.
```c
void writeCredentials() {
char *password = "cleartext password";
FILE* file = fopen("credentials.txt", "w");
// BAD: write password to disk in cleartext
fputs(password, file);
// GOOD: encrypt password first
char *encrypted = encrypt(password);
fputs(encrypted, file);
}
```
## References
* M. Dowd, J. McDonald and J. Schuhm, *The Art of Software Security Assessment*, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
* M. Howard and D. LeBlanc, *Writing Secure Code*, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
* Common Weakness Enumeration: [CWE-313](https://cwe.mitre.org/data/definitions/313.html).

67
docs/language/query-help/cpp/CleartextSqliteDatabase.md
View File

@@ -1,67 +0,0 @@
# Cleartext storage of sensitive information in an SQLite database
```
ID: cpp/cleartext-storage-database
Kind: path-problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-313
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-313/CleartextSqliteDatabase.ql)
Sensitive information that is stored in an unencrypted SQLite database is accessible to an attacker who gains access to the database.
## Recommendation
Ensure that if sensitive information is stored in a database then the database is always encrypted.
## Example
The following example shows two ways of storing information in an SQLite database. In the 'BAD' case, the credentials are simply stored in cleartext. In the 'GOOD' case, the database (and thus the credentials) are encrypted.
```c
void bad(void) {
char *password = "cleartext password";
sqlite3 *credentialsDB;
sqlite3_stmt *stmt;
if (sqlite3_open("credentials.db", &credentialsDB) == SQLITE_OK) {
// BAD: database opened without encryption being enabled
sqlite3_exec(credentialsDB, "CREATE TABLE IF NOT EXISTS creds (password TEXT);", NULL, NULL, NULL);
if (sqlite3_prepare_v2(credentialsDB, "INSERT INTO creds(password) VALUES(?)", -1, &stmt, NULL) == SQLITE_OK) {
sqlite3_bind_text(stmt, 1, password, -1, SQLITE_TRANSIENT);
sqlite3_step(stmt);
sqlite3_finalize(stmt);
sqlite3_close(credentialsDB);
}
}
}
void good(void) {
char *password = "cleartext password";
sqlite3 *credentialsDB;
sqlite3_stmt *stmt;
if (sqlite3_open("credentials.db", &credentialsDB) == SQLITE_OK) {
// GOOD: database encryption enabled:
sqlite3_exec(credentialsDB, "PRAGMA key = 'secretKey!'", NULL, NULL, NULL);
sqlite3_exec(credentialsDB, "CREATE TABLE IF NOT EXISTS creds (password TEXT);", NULL, NULL, NULL);
if (sqlite3_prepare_v2(credentialsDB, "INSERT INTO creds(password) VALUES(?)", -1, &stmt, NULL) == SQLITE_OK) {
sqlite3_bind_text(stmt, 1, password, -1, SQLITE_TRANSIENT);
sqlite3_step(stmt);
sqlite3_finalize(stmt);
sqlite3_close(credentialsDB);
}
}
}
```
## References
* M. Dowd, J. McDonald and J. Schuhm, *The Art of Software Security Assessment*, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
* M. Howard and D. LeBlanc, *Writing Secure Code*, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
* Common Weakness Enumeration: [CWE-313](https://cwe.mitre.org/data/definitions/313.html).

63
docs/language/query-help/cpp/ComparisonWithWiderType.md
View File

@@ -1,63 +0,0 @@
# Comparison of narrow type with wide type in loop condition
```
ID: cpp/comparison-with-wider-type
Kind: problem
Severity: warning
Precision: high
Tags: reliability security external/cwe/cwe-190 external/cwe/cwe-197 external/cwe/cwe-835
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.ql)
In a loop condition, comparison of a value of a narrow type with a value of a wide type may result in unexpected behavior if the wider value is sufficiently large (or small). This is because the narrower value may overflow. This can lead to an infinite loop.
## Recommendation
Change the types of the compared values so that the value on the narrower side of the comparison is at least as wide as the value it is being compared with.
## Example
In this example, `bytes_received` is compared against `max_get` in a `while` loop. However, `bytes_received` is an `int16_t`, and `max_get` is an `int32_t`. Because `max_get` is larger than `INT16_MAX`, the loop condition is always `true`, so the loop never terminates.
This problem is avoided in the 'GOOD' case because `bytes_received2` is an `int32_t`, which is as wide as the type of `max_get`.
```c
void main(int argc, char **argv) {
uint32_t big_num = INT32_MAX;
char buf[big_num];
int16_t bytes_received = 0;
int max_get = INT16_MAX + 1;
// BAD: 'bytes_received' is compared with a value of a wider type.
// 'bytes_received' overflows before reaching 'max_get',
// causing an infinite loop
while (bytes_received < max_get)
bytes_received += get_from_input(buf, bytes_received);
}
uint32_t bytes_received = 0;
// GOOD: 'bytes_received2' has a type at least as wide as 'max_get'
while (bytes_received < max_get) {
bytes_received += get_from_input(buf, bytes_received);
}
}
int getFromInput(char *buf, short pos) {
// write to buf
// ...
return 1;
}
```
## References
* [Data type ranges](https://docs.microsoft.com/en-us/cpp/cpp/data-type-ranges)
* [INT18-C. Evaluate integer expressions in a larger size before comparing or assigning to that size ](https://wiki.sei.cmu.edu/confluence/display/c/INT18-C.+Evaluate+integer+expressions+in+a+larger+size+before+comparing+or+assigning+to+that+size)
* Common Weakness Enumeration: [CWE-190](https://cwe.mitre.org/data/definitions/190.html).
* Common Weakness Enumeration: [CWE-197](https://cwe.mitre.org/data/definitions/197.html).
* Common Weakness Enumeration: [CWE-835](https://cwe.mitre.org/data/definitions/835.html).

56
docs/language/query-help/cpp/DangerousFunctionOverflow.md
View File

@@ -1,56 +0,0 @@
# Use of dangerous function
```
ID: cpp/dangerous-function-overflow
Kind: problem
Severity: error
Precision: very-high
Tags: reliability security external/cwe/cwe-242
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-676/DangerousFunctionOverflow.ql)
This rule finds calls to the `gets` function, which is dangerous and should not be used. See **Related rules** below for rules that identify other dangerous functions.
The `gets` 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 `gets` 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.
## Recommendation
Replace calls to `gets` with `fgets`, specifying the maximum length to copy. This will prevent the buffer overflow.
## Example
The following example gets a string from standard input in two ways:
```c
#define BUFFERSIZE (1024)
// BAD: using gets
void echo_bad() {
char buffer[BUFFERSIZE];
gets(buffer);
printf("Input was: '%s'\n", buffer);
}
// GOOD: using fgets
void echo_good() {
char buffer[BUFFERSIZE];
fgets(buffer, BUFFERSIZE, stdin);
printf("Input was: '%s'\n", buffer);
}
```
The first version uses `gets` and will overflow if the input is longer than the buffer. The second version of the code uses `fgets` and will not overflow, because the amount of data written is limited by the length parameter.
## Related rules
Other dangerous functions identified by CWE-676 ("Use of Potentially Dangerous Function") include `strcpy` and `strcat`. Use of these functions is highlighted by rules for the following CWEs:
* [CWE-120 Classic Buffer Overflow](https://cwe.mitre.org/data/definitions/120.html).
* [CWE-131 Incorrect Calculation of Buffer Size](https://cwe.mitre.org/data/definitions/131.html).
## References
* Wikipedia: [Morris worm](http://en.wikipedia.org/wiki/Morris_worm).
* E. Spafford. *The Internet Worm Program: An Analysis*. Purdue Technical Report CSD-TR-823, [(online)](http://www.textfiles.com/100/tr823.txt), 1988.
* Common Weakness Enumeration: [CWE-242](https://cwe.mitre.org/data/definitions/242.html).

47
docs/language/query-help/cpp/DangerousUseOfCin.md
View File

@@ -1,47 +0,0 @@
# Dangerous use of 'cin'
```
ID: cpp/dangerous-cin
Kind: problem
Severity: error
Precision: high
Tags: reliability security external/cwe/cwe-676
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-676/DangerousUseOfCin.ql)
This rule finds calls to `std::istream::operator>>` on `std::cin` without a preceding call to `cin.width`. Consuming input from `cin` without specifying the length of the input is dangerous due to the possibility of buffer overflows.
## Recommendation
Always specify the length of any input expected from `cin` by calling `cin.width` before consuming the input.
## Example
The following example shows both a dangerous and a safe way to consume input from `cin`.
```cpp
#define BUFFER_SIZE 20
void bad()
{
char buffer[BUFFER_SIZE];
// BAD: Use of 'cin' without specifying the length of the input.
cin >> buffer;
buffer[BUFFER_SIZE-1] = '\0';
}
void good()
{
char buffer[BUFFER_SIZE];
// GOOD: Specifying the length of the input before using 'cin'.
cin.width(BUFFER_SIZE);
cin >> buffer;
buffer[BUFFER_SIZE-1] = '\0';
}
```
## References
* Common Weakness Enumeration: [CWE-676](https://cwe.mitre.org/data/definitions/676.html).

45
docs/language/query-help/cpp/DoNotCreateWorldWritable.md
View File

@@ -1,45 +0,0 @@
# File created without restricting permissions
```
ID: cpp/world-writable-file-creation
Kind: problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-732
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-732/DoNotCreateWorldWritable.ql)
When you create a file, take care to give it the most restrictive permissions possible. A typical mistake is to create the file with world-writable permissions. This can allow an attacker to write to the file, which can give them unexpected control over the program.
## Recommendation
Files should usually be created with write permissions only for the current user. If broader permissions are needed, including the users' group should be sufficient. It is very rare that a file needs to be world-writable, and care should be taken not to make assumptions about the contents of any such file.
On Unix systems, it is possible for the user who runs the program to restrict file creation permissions using `umask`. However, a program should not assume that the user will set an `umask`, and should still set restrictive permissions by default.
## Example
This example shows two ways of writing a default configuration file. Software often does this to provide the user with a convenient starting point for defining their own configuration. However, configuration files can also control important aspects of the software's behavior, so it is important that they cannot be controlled by an attacker.
The first example creates the default configuration file with the usual "default" Unix permissions, `0666`. This makes the file world-writable, so that an attacker could write in their own configuration that would be read by the program. The second example uses more restrictive permissions: a combination of the standard Unix constants `S_IWUSR` and `S_IRUSR` which means that only the current user will have read and write access to the file.
```c
int write_default_config_bad() {
// BAD - this is world-writable so any user can overwrite the config
FILE* out = creat(OUTFILE, 0666);
fprintf(out, DEFAULT_CONFIG);
}
int write_default_config_good() {
// GOOD - this allows only the current user to modify the file
FILE* out = creat(OUTFILE, S_IWUSR | S_IRUSR);
fprintf(out, DEFAULT_CONFIG);
}
```
## References
* The CERT Oracle Secure Coding Standard for C: [ FIO06-C. Create files with appropriate access permissions ](https://www.securecoding.cert.org/confluence/display/c/FIO06-C.+Create+files+with+appropriate+access+permissions).
* Common Weakness Enumeration: [CWE-732](https://cwe.mitre.org/data/definitions/732.html).

42
docs/language/query-help/cpp/HResultBooleanConversion.md
View File

@@ -1,42 +0,0 @@
# Cast between HRESULT and a Boolean type
```
ID: cpp/hresult-boolean-conversion
Kind: problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-253 external/microsoft/C6214 external/microsoft/C6215 external/microsoft/C6216 external/microsoft/C6217 external/microsoft/C6230
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.ql)
This query indicates that an `HRESULT` is being cast to a Boolean type or vice versa.
The typical success value (`S_OK`) of an `HRESULT` equals 0. However, 0 indicates failure for a Boolean type.
Casting an `HRESULT` to a Boolean type and then using it in a test expression will yield an incorrect result.
## Recommendation
To check if a call that returns an `HRESULT` succeeded use the `FAILED` macro.
## Example
In the following example, `HRESULT` is used in a test expression incorrectly as it may yield an incorrect result.
```cpp
LPMALLOC pMalloc;
HRESULT hr = CoGetMalloc(1, &pMalloc);
if (!hr)
{
// code ...
}
```
To fix this issue, use the `FAILED` macro in the test expression.
## References
* Common Weakness Enumeration: [CWE-253](https://cwe.mitre.org/data/definitions/253.html).

55
docs/language/query-help/cpp/IncorrectNotOperatorUsage.md
View File

@@ -1,55 +0,0 @@
# Incorrect 'not' operator usage
```
ID: cpp/incorrect-not-operator-usage
Kind: problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-480 external/microsoft/c6317
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Likely%20Typos/IncorrectNotOperatorUsage.ql)
This rule finds logical-not operator usage as an operator for in a bit-wise operation.
Due to the nature of logical operation result value, only the lowest bit could possibly be set, and it is unlikely to be intent in bitwise opeartions. Violations are often indicative of a typo, using a logical-not (`!`) opeartor instead of the bit-wise not (`~`) operator.
This rule is restricted to analyze bit-wise and (`&`) and bit-wise or (`|`) operation in order to provide better precision.
This rule ignores instances where a double negation (`!!`) is explicitly used as the opeartor of the bitwise operation, as this is a commonly used as a mechanism to normalize an integer value to either 1 or 0.
NOTE: It is not recommended to use this rule in kernel code or older C code as it will likely find several false positive instances.
## Recommendation
Carefully inspect the flagged expressions. Consider the intent in the code logic, and decide whether it is necessary to change the not operator.
## Example
```cpp
#define FLAGS 0x4004
void f_warning(int i)
{
// The usage of the logical not operator in this case is unlikely to be correct
// as the output is being used as an operator for a bit-wise and operation
if (i & !FLAGS)
{
// code
}
}
void f_fixed(int i)
{
if (i & ~FLAGS) // Changing the logical not operator for the bit-wise not operator would fix this logic
{
// code
}
}
```
## References
* [warning C6317: incorrect operator: logical-not (!) is not interchangeable with ones-complement (~)](https://docs.microsoft.com/en-us/visualstudio/code-quality/c6317?view=vs-2017)
* Common Weakness Enumeration: [CWE-480](https://cwe.mitre.org/data/definitions/480.html).

43
docs/language/query-help/cpp/IncorrectPointerScaling.md
View File

@@ -1,43 +0,0 @@
# Suspicious pointer scaling
```
ID: cpp/suspicious-pointer-scaling
Kind: problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-468
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScaling.ql)
Pointer arithmetic in C and C++ is automatically scaled according to the size of the data type. For example, if the type of `p` is `T*` and `sizeof(T) == 4` then the expression `p+1` adds 4 bytes to `p`. This can cause a buffer overflow condition if the programmer forgets that they are adding a multiple of `sizeof(T)`, rather than a number of bytes.
This query finds pointer arithmetic expressions where it appears likely that the programmer has forgotten that the offset is automatically scaled.
## Recommendation
1. Whenever possible, use the array subscript operator rather than pointer arithmetic. For example, replace `*(p+k)` with `p[k]`.
1. Cast to the correct type before using pointer arithmetic. For example, if the type of `p` is `int*` but it really points to an array of type `double[]` then use the syntax `(double*)p + k` to get a pointer to the `k`'th element of the array.
## Example
```cpp
int example1(int i) {
int intArray[10] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
int *intPointer = intArray;
// BAD: the offset is already automatically scaled by sizeof(int),
// so this code will compute the wrong offset.
return *(intPointer + (i * sizeof(int)));
}
int example2(int i) {
int intArray[10] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
int *intPointer = intArray;
// GOOD: the offset is automatically scaled by sizeof(int).
return *(intPointer + i);
}
```
## References
* Common Weakness Enumeration: [CWE-468](https://cwe.mitre.org/data/definitions/468.html).

44
docs/language/query-help/cpp/IncorrectPointerScalingVoid.md
View File

@@ -1,44 +0,0 @@
# Suspicious pointer scaling to void
```
ID: cpp/suspicious-pointer-scaling-void
Kind: problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-468
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-468/IncorrectPointerScalingVoid.ql)
Casting arbitrary pointers into `void*` and then accessing their contents should be done with care. The results may not be portable.
This query finds pointer arithmetic expressions where a pointer to `void` (or similar) is then cast to another type and dereferenced.
## Recommendation
1. Whenever possible, use the array subscript operator rather than pointer arithmetic. For example, replace `*(p+k)` with `p[k]`.
1. Cast to the correct type before using pointer arithmetic. For example, if the type of `p` is `void*` but it really points to an array of type `double[]` then use the syntax `(double*)p + k` to get a pointer to the `k`'th element of the array.
1. If pointer arithmetic must be done with a single-byte width, prefer `char *` to `void *`, as pointer arithmetic on `void *` is a nonstandard GNU extension.
## Example
```cpp
char example1(int i) {
int intArray[5] = { 1, 2, 3, 4, 5 };
void *voidPointer = (void *)intArray;
// BAD: the pointer arithmetic uses type void*, so the offset
// is not scaled by sizeof(int).
return *(voidPointer + i);
}
int example2(int i) {
int intArray[10] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
int *intPointer = intArray;
// GOOD: the offset is automatically scaled by sizeof(int).
return *(intPointer + i);
}
```
## References
* Common Weakness Enumeration: [CWE-468](https://cwe.mitre.org/data/definitions/468.html).

40
docs/language/query-help/cpp/IntMultToLong.md
View File

@@ -1,40 +0,0 @@
# Multiplication result converted to larger type
```
ID: cpp/integer-multiplication-cast-to-long
Kind: problem
Severity: warning
Precision: high
Tags: reliability security correctness types external/cwe/cwe-190 external/cwe/cwe-192 external/cwe/cwe-197 external/cwe/cwe-681
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Arithmetic/IntMultToLong.ql)
This rule finds code that converts the result of an integer multiplication to a larger type. Since the conversion applies *after* the multiplication, arithmetic overflow may still occur.
The rule flags every multiplication of two non-constant integer expressions that is (explicitly or implicitly) converted to a larger integer type. The conversion is an indication that the expression would produce a result that would be too large to fit in the smaller integer type.
## Recommendation
Use a cast to ensure that the multiplication is done using the larger integer type to avoid overflow.
## Example
```cpp
int i = 2000000000;
long j = i * i; //Wrong: due to overflow on the multiplication between ints,
//will result to j being -1651507200, not 4000000000000000000
long k = (long) i * i; //Correct: the multiplication is done on longs instead of ints,
//and will not overflow
```
## References
* MSDN Library: [Multiplicative Operators: *, /, and %](http://msdn.microsoft.com/en-us/library/ty2ax9z9%28v=vs.71%29.aspx).
* Cplusplus.com: [Integer overflow](http://www.cplusplus.com/articles/DE18T05o/).
* Common Weakness Enumeration: [CWE-190](https://cwe.mitre.org/data/definitions/190.html).
* Common Weakness Enumeration: [CWE-192](https://cwe.mitre.org/data/definitions/192.html).
* Common Weakness Enumeration: [CWE-197](https://cwe.mitre.org/data/definitions/197.html).
* Common Weakness Enumeration: [CWE-681](https://cwe.mitre.org/data/definitions/681.html).

34
docs/language/query-help/cpp/NewFreeMismatch.md
View File

@@ -1,34 +0,0 @@
# Mismatching new/free or malloc/delete
```
ID: cpp/new-free-mismatch
Kind: problem
Severity: warning
Precision: high
Tags: reliability security external/cwe/cwe-401
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Critical/NewFreeMismatch.ql)
This rule finds `delete` expressions whose argument is a pointer that points to memory allocated using the `malloc` function, and calls to `free` whose argument is a pointer that points to memory allocated using the `new` operator. Behavior in such cases is undefined and should be avoided.
## Recommendation
Use the `delete` operator when freeing memory allocated with `new`, and the `free` function when freeing memory allocated with `malloc`.
## Example
```cpp
Record *ptr = new Record(...);
...
free(ptr); // BAD: ptr was created using 'new', but is being freed using 'free'
```
## References
* isocpp.org 'Standard C++', "[Can I free() pointers allocated with new? Can I delete pointers allocated with malloc()?](https://isocpp.org/wiki/faq/freestore-mgmt#mixing-malloc-and-delete)"
* Wikipedia, "[Relation to malloc and free](https://en.wikipedia.org/wiki/New_and_delete_(C%2B%2B)#Relation_to_malloc_and_free)" in *new and delete (C++)*.
* Common Weakness Enumeration: [CWE-401](https://cwe.mitre.org/data/definitions/401.html).

43
docs/language/query-help/cpp/NoSpaceForZeroTerminator.md
View File

@@ -1,43 +0,0 @@
# No space for zero terminator
```
ID: cpp/no-space-for-terminator
Kind: problem
Severity: error
Precision: high
Tags: reliability security external/cwe/cwe-131 external/cwe/cwe-120 external/cwe/cwe-122
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql)
This rule identifies calls to `malloc` that call `strlen` to determine the required buffer size, but do not allocate space for the zero terminator.
## Recommendation
The expression highlighted by this rule creates a buffer that is of insufficient size to contain the data being copied. This makes the code vulnerable to buffer overflow which can result in anything from a segmentation fault to a security vulnerability (particularly if the array is on stack-allocated memory).
Increase the size of the buffer being allocated by one or replace `malloc`, `strcpy` pairs with a call to `strdup`
## Example
```c
void flawed_strdup(const char *input)
{
char *copy;
/* Fail to allocate space for terminating '\0' */
copy = (char *)malloc(strlen(input));
strcpy(copy, input);
return copy;
}
```
## References
* CERT C Coding Standard: [MEM35-C. Allocate sufficient memory for an object](https://www.securecoding.cert.org/confluence/display/c/MEM35-C.+Allocate+sufficient+memory+for+an+object).
* Common Weakness Enumeration: [CWE-131](https://cwe.mitre.org/data/definitions/131.html).
* Common Weakness Enumeration: [CWE-120](https://cwe.mitre.org/data/definitions/120.html).
* Common Weakness Enumeration: [CWE-122](https://cwe.mitre.org/data/definitions/122.html).

109
docs/language/query-help/cpp/NonConstantFormat.md
View File

@@ -1,109 +0,0 @@
# Non-constant format string
```
ID: cpp/non-constant-format
Kind: problem
Severity: recommendation
Precision: high
Tags: maintainability correctness security external/cwe/cwe-134
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Format/NonConstantFormat.ql)
The `printf` function, related functions like `sprintf` and `fprintf`, and other functions built atop `vprintf` all accept a format string as one of their arguments. When such format strings are literal constants, it is easy for the programmer (and static analysis tools) to verify that the format specifiers (such as `%s` and `%02x`) in the format string are compatible with the trailing arguments of the function call. When such format strings are not literal constants, it is more difficult to maintain the program: programmers (and static analysis tools) must perform non-local data-flow analysis to deduce what values the format string argument might take.
## Recommendation
If the argument passed as a format string is meant to be a plain string rather than a format string, then pass `%s` as the format string, and pass the original argument as the sole trailing argument.
If the argument passed as a format string is a parameter to the enclosing function, then consider redesigning the enclosing function's API to be less brittle.
## Example
The following program is meant to echo its command line arguments:
```c
#include <stdio.h>
int main(int argc, char** argv) {
for(int i = 1; i < argc; ++i) {
printf(argv[i]);
}
}
```
The above program behaves as expected in most cases, but breaks when one of its command line arguments contains a percent character. In such cases, the behavior of the program is undefined: it might echo garbage, it might crash, or it might give a malicious attacker root access. One way of addressing the problem is to use a constant `%s` format string, as in the following program:
```c
#include <stdio.h>
int main(int argc, char** argv) {
for(int i = 1; i < argc; ++i) {
printf("%s", argv[i]);
}
}
```
## Example
The following program defines a `log_with_timestamp` function:
```c
void log_with_timestamp(const char* message) {
struct tm now;
time(&now);
printf("[%s] ", asctime(now));
printf(message);
}
int main(int argc, char** argv) {
log_with_timestamp("Application is starting...\n");
/* ... */
log_with_timestamp("Application is closing...\n");
return 0;
}
```
In the code that is visible, the reader can verify that `log_with_timestamp` is never called with a log message containing a percent character, but even if all current calls are correct, this presents an ongoing maintenance burden to ensure that newly-introduced calls don't contain percent characters. As in the previous example, one solution is to make the log message a trailing argument of the function call:
```c
void log_with_timestamp(const char* message) {
struct tm now;
time(&now);
printf("[%s] %s", asctime(now), message);
}
int main(int argc, char** argv) {
log_with_timestamp("Application is starting...\n");
/* ... */
log_with_timestamp("Application is closing...\n");
return 0;
}
```
An alternative solution is to allow `log_with_timestamp` to accept format arguments:
```c
void log_with_timestamp(const char* message, ...) {
va_list args;
va_start(args, message);
struct tm now;
time(&now);
printf("[%s] ", asctime(now));
vprintf(message, args);
va_end(args);
}
int main(int argc, char** argv) {
log_with_timestamp("%s is starting...\n", argv[0]);
/* ... */
log_with_timestamp("%s is closing...\n", argv[0]);
return 0;
}
```
In this formulation, the non-constant format string to `printf` has been replaced with a non-constant format string to `vprintf`. Semmle will no longer consider the body of `log_with_timestamp` to be a problem, and will instead check that every call to `log_with_timestamp` passes a constant format string.
## References
* CERT C Coding Standard: [FIO30-C. Exclude user input from format strings](https://www.securecoding.cert.org/confluence/display/c/FIO30-C.+Exclude+user+input+from+format+strings).
* M. Howard, D. Leblanc, J. Viega, *19 Deadly Sins of Software Security: Programming Flaws and How to Fix Them*.
* Common Weakness Enumeration: [CWE-134](https://cwe.mitre.org/data/definitions/134.html).

58
docs/language/query-help/cpp/OffsetUseBeforeRangeCheck.md
View File

@@ -1,58 +0,0 @@
# Array offset used before range check
```
ID: cpp/offset-use-before-range-check
Kind: problem
Severity: warning
Precision: medium
Tags: reliability security external/cwe/cwe-120 external/cwe/cwe-125
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Best%20Practices/Likely%20Errors/OffsetUseBeforeRangeCheck.ql)
The program contains an and-expression where the array access is defined before the range check. Consequently the array is accessed without any bounds checking. The range check does not protect the program from segmentation faults caused by attempts to read beyond the end of a buffer.
## Recommendation
Update the and-expression so that the range check precedes the array offset. This will ensure that the bounds are checked before the array is accessed.
## Example
The `find` function can read past the end of the buffer pointed to by `str` if `start` is longer than or equal to the length of the buffer (or longer than `len`, depending on the contents of the buffer).
```c
int find(int start, char *str, char goal)
{
int len = strlen(str);
//Potential buffer overflow
for (int i = start; str[i] != 0 && i < len; i++) {
if (str[i] == goal)
return i;
}
return -1;
}
int findRangeCheck(int start, char *str, char goal)
{
int len = strlen(str);
//Range check protects against buffer overflow
for (int i = start; i < len && str[i] != 0 ; i++) {
if (str[i] == goal)
return i;
}
return -1;
}
```
Update the and-expression so that the range check precedes the array offset (for example, the `findRangeCheck` function).
## References
* cplusplus.com: [ C++: array](http://www.cplusplus.com/reference/array/array/).
* Wikipedia: [ Bounds checking](http://en.wikipedia.org/wiki/Bounds_checking).
* Common Weakness Enumeration: [CWE-120](https://cwe.mitre.org/data/definitions/120.html).
* Common Weakness Enumeration: [CWE-125](https://cwe.mitre.org/data/definitions/125.html).

60
docs/language/query-help/cpp/OpenSslHeartbleed.md
View File

@@ -1,60 +0,0 @@
# Use of a version of OpenSSL with Heartbleed
```
ID: cpp/openssl-heartbleed
Kind: problem
Severity: error
Precision: very-high
Tags: security external/cwe/cwe-327 external/cwe/cwe-788
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-327/OpenSslHeartbleed.ql)
Earlier versions of the popular OpenSSL library suffer from a buffer overflow in its "heartbeat" code. Because of the location of the problematic code, this vulnerability is often called "Heartbleed".
Software that includes a copy of OpenSSL should be sure to use a current version of the library. If it uses an older version, it will be vulnerable to any network site it connects with.
## Recommendation
Upgrade to the latest version of OpenSSL. This problem was fixed in version 1.0.1g.
## Example
The following code is present in earlier versions of OpenSSL. The `payload` variable is the number of bytes that should be copied from the request back into the response. The call to `memcpy` does this copy. The problem is that `payload` is supplied as part of the remote request, and there is no code that checks the size of it. If the caller supplies a very large value, then the `memcpy` call will copy memory that is outside the request packet.
```c
int
tls1_process_heartbeat(SSL *s)
{
unsigned char *p = &s->s3->rrec.data[0], *pl;
unsigned short hbtype;
unsigned int payload;
/* ... */
hbtype = *p++;
n2s(p, payload);
pl = p;
/* ... */
if (hbtype == TLS1_HB_REQUEST)
{
/* ... */
memcpy(bp, pl, payload); // BAD: overflow here
/* ... */
}
/* ... */
}
```
## References
* Common Vulnerabilities and Exposures: [CVE-2014-0160](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-0160).
* OpenSSL News: [OpenSSL Security Advisory [07 Apr 2014]](https://www.openssl.org/news/secadv_20140407.txt).
* Common Weakness Enumeration: [CWE-327](https://cwe.mitre.org/data/definitions/327.html).
* Common Weakness Enumeration: [CWE-788](https://cwe.mitre.org/data/definitions/788.html).

41
docs/language/query-help/cpp/OverflowStatic.md
View File

@@ -1,41 +0,0 @@
# Static array access may cause overflow
```
ID: cpp/static-buffer-overflow
Kind: problem
Severity: warning
Precision: medium
Tags: reliability security external/cwe/cwe-119 external/cwe/cwe-131
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Critical/OverflowStatic.ql)
When you use static arrays you must ensure that you do not exceed the size of the array during write and access operations. If an operation attempts to write to or access an element that is outside the range of the array then this results in a buffer overflow. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.
## Recommendation
Check the offsets and sizes used in the highlighted operations to ensure that a buffer overflow will not occur.
## Example
```cpp
#define SIZE 30
int f(char * s) {
char buf[20]; //buf not set to use SIZE macro
strncpy(buf, s, SIZE); //wrong: copy may exceed size of buf
for (int i = 0; i < SIZE; i++) { //wrong: upper limit that is higher than array size
cout << array[i];
}
}
```
## References
* I. Gerg. *An Overview and Example of the Buffer-Overflow Exploit*. IANewsletter vol 7 no 4. 2005.
* M. Donaldson. *Inside the Buffer Overflow Attack: Mechanism, Method & Prevention*. SANS Institute InfoSec Reading Room. 2002.
* Common Weakness Enumeration: [CWE-119](https://cwe.mitre.org/data/definitions/119.html).
* Common Weakness Enumeration: [CWE-131](https://cwe.mitre.org/data/definitions/131.html).

45
docs/language/query-help/cpp/OverrunWrite.md
View File

@@ -1,45 +0,0 @@
# Potentially overrunning write
```
ID: cpp/overrunning-write
Kind: problem
Severity: error
Precision: medium
Tags: reliability security external/cwe/cwe-120 external/cwe/cwe-787 external/cwe/cwe-805
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-120/OverrunWrite.ql)
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.
## Recommendation
Always control the length of buffer copy and buffer write operations. `strncpy` should be used over `strcpy`, `snprintf` over `sprintf`, and in other cases 'n-variant' functions should be preferred.
## Example
```c
void sayHello()
{
char buffer[10];
// BAD: this message overflows the buffer
strcpy(buffer, "Hello, world!");
MessageBox(hWnd, buffer, "New Message", MB_OK);
}
```
In this example, the call to `strcpy` copies a message of 14 characters (including the terminating null) into a buffer with space for just 10 characters. As such, the last four characters overflow the buffer resulting in undefined behavior.
To fix this issue three changes should be made:
* Control the size of the buffer using a preprocessor define.
* Replace the call to `strcpy` with `strncpy`, specifying the define as the maximum length to copy. This will prevent the buffer overflow.
* Consider increasing the buffer size, say to 20 characters, so that the message is displayed correctly.
## References
* CERT C Coding Standard: [STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator](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).
* Common Weakness Enumeration: [CWE-120](https://cwe.mitre.org/data/definitions/120.html).
* Common Weakness Enumeration: [CWE-787](https://cwe.mitre.org/data/definitions/787.html).
* Common Weakness Enumeration: [CWE-805](https://cwe.mitre.org/data/definitions/805.html).

45
docs/language/query-help/cpp/OverrunWriteFloat.md
View File

@@ -1,45 +0,0 @@
# Potentially overrunning write with float to string conversion
```
ID: cpp/overrunning-write-with-float
Kind: problem
Severity: error
Precision: medium
Tags: reliability security external/cwe/cwe-120 external/cwe/cwe-787 external/cwe/cwe-805
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-120/OverrunWriteFloat.ql)
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.
## Recommendation
Always control the length of buffer copy and buffer write operations. `strncpy` should be used over `strcpy`, `snprintf` over `sprintf`, and in other cases 'n-variant' functions should be preferred.
## Example
```c
void displayValue(double value)
{
char buffer[256];
// BAD: extreme values may overflow the buffer
sprintf(buffer, "%f", value);
MessageBox(hWnd, buffer, "A Number", MB_OK);
}
```
In this example, the call to `sprintf` 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.
To fix this issue three changes should be made:
* Control the size of the buffer using a preprocessor define.
* Replace the call to `sprintf` with `snprintf`, specifying the define as the maximum length to copy. This will prevent the buffer overflow.
* Consider using the %g format specifier instead of %f.
## References
* CERT C Coding Standard: [STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator](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).
* Common Weakness Enumeration: [CWE-120](https://cwe.mitre.org/data/definitions/120.html).
* Common Weakness Enumeration: [CWE-787](https://cwe.mitre.org/data/definitions/787.html).
* Common Weakness Enumeration: [CWE-805](https://cwe.mitre.org/data/definitions/805.html).

45
docs/language/query-help/cpp/PointerOverflow.md
View File

@@ -1,45 +0,0 @@
# Pointer overflow check
```
ID: cpp/pointer-overflow-check
Kind: problem
Severity: error
Precision: high
Tags: reliability security
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Memory%20Management/PointerOverflow.ql)
When checking for integer overflow, you may often write tests like `p + i < p`. This works fine if `p` and `i` are unsigned integers, since any overflow in the addition will cause the value to simply "wrap around." However, using this pattern when `p` is a pointer is problematic because pointer overflow has undefined behavior according to the C and C++ standards. If the addition overflows and has an undefined result, the comparison will likewise be undefined; it may produce an unintended result, or may be deleted entirely by an optimizing compiler.
## Recommendation
To check whether an index `i` is less than the length of an array, simply compare these two numbers as unsigned integers: `i < ARRAY_LENGTH`. If the length of the array is defined as the difference between two pointers `ptr` and `p_end`, write `i < p_end - ptr`. If `i` is signed, cast it to unsigned in order to guard against negative `i`. For example, write `(size_t)i < p_end - ptr`.
## Example
An invalid check for pointer overflow is most often seen as part of checking whether a number `a` is too large by checking first if adding the number to `ptr` goes past the end of an allocation and then checking if adding it to `ptr` creates a pointer so large that it overflows and wraps around.
```cpp
bool not_in_range(T *ptr, T *ptr_end, size_t i) {
return ptr + i >= ptr_end || ptr + i < ptr; // BAD
}
```
In both of these checks, the operations are performed in the wrong order. First, an expression that may cause undefined behavior is evaluated (`ptr + i`), and then the result is checked for being in range. But once undefined behavior has happened in the pointer addition, it cannot be recovered from: it's too late to perform the range check after a possible pointer overflow.
While it's not the subject of this query, the expression `ptr + i < ptr_end` is also an invalid range check. It's undefined behavor in C/C++ to create a pointer that points more than one past the end of an allocation.
The next example shows how to portably check whether an unsigned number is outside the range of an allocation between `ptr` and `ptr_end`.
```cpp
bool not_in_range(T *ptr, T *ptr_end, size_t i) {
return i >= ptr_end - ptr; // GOOD
}
```
## References
* Embedded in Academia: [Pointer Overflow Checking](https://blog.regehr.org/archives/1395).
* LWN: [GCC and pointer overflows](https://lwn.net/Articles/278137/).

50
docs/language/query-help/cpp/PotentiallyDangerousFunction.md
View File

@@ -1,50 +0,0 @@
# Use of potentially dangerous function
```
ID: cpp/potentially-dangerous-function
Kind: problem
Severity: warning
Precision: high
Tags: reliability security external/cwe/cwe-676
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql)
This rule finds calls to functions that are dangerous to use. Currently, it checks for calls to `gmtime`, `localtime`, `ctime` and `asctime`.
The time related functions such as `gmtime` fill data into a `tm` struct or `char` 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.
## Recommendation
Replace calls to `gmtime` with `gmtime_r`. With `gmtime_r`, the application code manages allocation of the `tm` struct. That way, separate calls to the function can use their own storage.
Similarly replace calls to `localtime` with `localtime_r`, calls to `ctime` with `ctime_r` and calls to `asctime` with `asctime_r`.
## Example
The following example checks the local time in two ways:
```c
// 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);
}
// 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);
}
```
The first version uses `gmtime`, 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 `gmtime_r`. Since it allocates a new `tm` struct on every call, it is immune to other calls to `gmtime` or `gmtime_r`.
## References
* SEI CERT C Coding Standard: [CON33-C. Avoid race conditions when using library functions](https://wiki.sei.cmu.edu/confluence/display/c/CON33-C.+Avoid+race+conditions+when+using+library+functions).
* Common Weakness Enumeration: [CWE-676](https://cwe.mitre.org/data/definitions/676.html).

70
docs/language/query-help/cpp/SignedOverflowCheck.md
View File

@@ -1,70 +0,0 @@
# Signed overflow check
```
ID: cpp/signed-overflow-check
Kind: problem
Severity: warning
Precision: high
Tags: correctness security
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Arithmetic/SignedOverflowCheck.ql)
When checking for integer overflow, you may often write tests like `a + b < a`. This works fine if `a` or `b` are unsigned integers, since any overflow in the addition will cause the value to simply "wrap around." However, using *signed* integers is problematic because signed overflow has undefined behavior according to the C and C++ standards. If the addition overflows and has an undefined result, the comparison will likewise be undefined; it may produce an unintended result, or may be deleted entirely by an optimizing compiler.
## Recommendation
Solutions to this problem can be thought of as falling into one of two categories:
1. Rewrite the signed expression so that overflow cannot occur but the signedness remains.
1. Change the variables and all their uses to be unsigned.
The following cases all fall into the first category.
1. Given `unsigned short n1, delta` and `n1 + delta < n1`, it is possible to rewrite it as `(unsigned short)(n1 + delta)&nbsp;<&nbsp;n1`. Note that `n1 + delta` does not actually overflow, due to `int` promotion.
1. Given `unsigned short n1, delta` and `n1 + delta < n1`, it is also possible to rewrite it as `n1 > USHORT_MAX - delta`. The `limits.h` or `climits` header must then be included.
1. Given `int n1, delta` and `n1 + delta < n1`, it is possible to rewrite it as `n1 > INT_MAX - delta`. It must be true that `delta >= 0` and the `limits.h` or `climits` header has been included.
## Example
In the following example, even though `delta` has been declared `unsigned short`, C/C++ type promotion rules require that its type is promoted to the larger type used in the addition and comparison, namely a `signed int`. Addition is performed on signed integers, and may have undefined behavior if an overflow occurs. As a result, the entire (comparison) expression may also have an undefined result.
```cpp
bool foo(int n1, unsigned short delta) {
return n1 + delta < n1; // BAD
}
```
The following example builds upon the previous one. Instead of performing an addition (which could overflow), we have re-framed the solution so that a subtraction is used instead. Since `delta` is promoted to a `signed int` and `INT_MAX` denotes the largest possible positive value for an `signed int`, the expression `INT_MAX - delta` can never be less than zero or more than `INT_MAX`. Hence, any overflow and underflow are avoided.
```cpp
#include <limits.h>
bool foo(int n1, unsigned short delta) {
return n1 > INT_MAX - delta; // GOOD
}
```
In the following example, even though both `n` and `delta` have been declared `unsigned short`, both are promoted to `signed int` prior to addition. Because we started out with the narrower `short` type, the addition is guaranteed not to overflow and is therefore defined. But the fact that `n1 + delta` never overflows means that the condition `n1 + delta < n1` will never hold true, which likely is not what the programmer intended. (see also the `cpp/bad-addition-overflow-check` query).
```cpp
bool bar(unsigned short n1, unsigned short delta) {
// NB: Comparison is always false
return n1 + delta < n1; // GOOD (but misleading)
}
```
The next example provides a solution to the previous one. Even though `n1 + delta` does not overflow, casting it to an `unsigned short` truncates the addition modulo 2^16, so that `unsigned short` "wrap around" may now be observed. Furthermore, since the left-hand side is now of type `unsigned short`, the right-hand side does not need to be promoted to a `signed int`.
```cpp
bool bar(unsigned short n1, unsigned short delta) {
return (unsigned short)(n1 + delta) < n1; // GOOD
}
```
## References
* [comp.lang.c FAQ list <20> Question 3.19 (Preserving rules)](http://c-faq.com/expr/preservingrules.html)
* [INT31-C. Ensure that integer conversions do not result in lost or misinterpreted data](https://wiki.sei.cmu.edu/confluence/display/c/INT31-C.+Ensure+that+integer+conversions+do+not+result+in+lost+or+misinterpreted+data)
* W. Dietz, P. Li, J. Regehr, V. Adve. [Understanding Integer Overflow in C/C++](https://www.cs.utah.edu/~regehr/papers/overflow12.pdf)

40
docs/language/query-help/cpp/SizeCheck.md
View File

@@ -1,40 +0,0 @@
# Not enough memory allocated for pointer type
```
ID: cpp/allocation-too-small
Kind: problem
Severity: warning
Precision: medium
Tags: reliability security external/cwe/cwe-131 external/cwe/cwe-122
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Critical/SizeCheck.ql)
When you allocate an array from memory using `malloc`, `calloc` or `realloc`, you should ensure that you allocate enough memory to contain an instance of the required pointer type. Calls that are assigned to a non-void pointer variable, but do not allocate enough memory will cause a buffer overflow when a field accessed on the pointer points to memory that is beyond the allocated array. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.
## Recommendation
The highlighted call allocates memory that is too small to contain an instance of the type of the pointer, which can cause a memory overrun. Use the `sizeof` operator to ensure that the function call allocates enough memory for that type.
## Example
```cpp
#define RECORD_SIZE 30 //incorrect or outdated size for record
typedef struct {
char name[30];
int status;
} Record;
void f() {
Record* p = malloc(RECORD_SIZE); //not of sufficient size to hold a Record
...
}
```
## References
* I. Gerg. *An Overview and Example of the Buffer-Overflow Exploit*. IANewsletter vol 7 no 4. 2005.
* M. Donaldson. *Inside the Buffer Overflow Attack: Mechanism, Method & Prevention*. SANS Institute InfoSec Reading Room. 2002.
* Common Weakness Enumeration: [CWE-131](https://cwe.mitre.org/data/definitions/131.html).
* Common Weakness Enumeration: [CWE-122](https://cwe.mitre.org/data/definitions/122.html).

41
docs/language/query-help/cpp/SizeCheck2.md
View File

@@ -1,41 +0,0 @@
# Not enough memory allocated for array of pointer type
```
ID: cpp/suspicious-allocation-size
Kind: problem
Severity: warning
Precision: medium
Tags: reliability security external/cwe/cwe-131 external/cwe/cwe-122
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Critical/SizeCheck2.ql)
When you allocate an array from memory using `malloc`, `calloc` or `realloc`, you should ensure that you allocate enough memory to contain a multiple of the size of the required pointer type. Calls that are assigned to a non-void pointer variable, but do not allocate enough memory will cause a buffer overflow when a field accessed on the pointer points to memory that is beyond the allocated array. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.
## Recommendation
The highlighted call allocates memory that is not a multiple of the size of the pointer type, which can cause a memory overrun. Use the `sizeof` operator to ensure that the function call allocates enough memory for that type.
## Example
```cpp
#define RECORD_SIZE 30 //incorrect or outdated size for record
typedef struct {
char name[30];
int status;
} Record;
void f() {
Record* p = malloc(RECORD_SIZE * 4); //wrong: not a multiple of the size of Record
p[3].status = 1; //will most likely segfault
...
}
```
## References
* I. Gerg. *An Overview and Example of the Buffer-Overflow Exploit*. IANewsletter vol 7 no 4. 2005.
* M. Donaldson. *Inside the Buffer Overflow Attack: Mechanism, Method & Prevention*. SANS Institute InfoSec Reading Room. 2002.
* Common Weakness Enumeration: [CWE-131](https://cwe.mitre.org/data/definitions/131.html).
* Common Weakness Enumeration: [CWE-122](https://cwe.mitre.org/data/definitions/122.html).

66
docs/language/query-help/cpp/SnprintfOverflow.md
View File

@@ -1,66 +0,0 @@
# Potentially overflowing call to snprintf
```
ID: cpp/overflowing-snprintf
Kind: problem
Severity: warning
Precision: high
Tags: reliability correctness security
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Format/SnprintfOverflow.ql)
The return value of a call to `snprintf` is the number of characters that *would have* been written to the buffer assuming there was sufficient space. In the event that the operation reaches the end of the buffer and more than one character is discarded, the return value will be greater than the buffer size. This can cause incorrect behaviour, for example:
## Example
```cpp
#define BUF_SIZE (32)
int main(int argc, char *argv[])
{
char buffer[BUF_SIZE];
size_t pos = 0;
int i;
for (i = 0; i < argc; i++)
{
pos += snprintf(buffer + pos, BUF_SIZE - pos, "%s", argv[i]);
// BUF_SIZE - pos may overflow
}
}
```
## Recommendation
The return value of `snprintf` should always be checked if it is used, and values larger than the buffer size should be accounted for.
## Example
```cpp
#define BUF_SIZE (32)
int main(int argc, char *argv[])
{
char buffer[BUF_SIZE];
size_t pos = 0;
int i;
for (i = 0; i < argc; i++)
{
int n = snprintf(buffer + pos, BUF_SIZE - pos, "%s", argv[i]);
if (n < 0 || n >= BUF_SIZE - pos)
{
break;
}
pos += n;
}
}
```
## References
* cplusplus.com: [snprintf](http://www.cplusplus.com/reference/cstdio/snprintf/).
* Red Hat Customer Portal: [The trouble with snprintf](https://access.redhat.com/blogs/766093/posts/1976193).

43
docs/language/query-help/cpp/SqlTainted.md
View File

@@ -1,43 +0,0 @@
# Uncontrolled data in SQL query
```
ID: cpp/sql-injection
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-089
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql)
The code passes user input as part of a SQL query without escaping special elements. It generates a SQL query using `sprintf`, with the user-supplied data directly passed as an argument to `sprintf`. This leaves the code vulnerable to attack by SQL Injection.
## Recommendation
Use a library routine to escape characters in the user-supplied string before converting it to SQL.
## Example
```c
int main(int argc, char** argv) {
char *userName = argv[2];
// BAD
char query1[1000] = {0};
sprintf(query1, "SELECT UID FROM USERS where name = \"%s\"", userName);
runSql(query1);
// GOOD
char userNameSql[1000] = {0};
encodeSqlString(userNameSql, 1000, userName);
char query2[1000] = {0};
sprintf(query2, "SELECT UID FROM USERS where name = \"%s\"", userNameSql);
runSql(query2);
}
```
## References
* Microsoft Developer Network: [SQL Injection](http://msdn.microsoft.com/en-us/library/ms161953.aspx).
* Common Weakness Enumeration: [CWE-89](https://cwe.mitre.org/data/definitions/89.html).

34
docs/language/query-help/cpp/StrncpyFlippedArgs.md
View File

@@ -1,34 +0,0 @@
# Possibly wrong buffer size in string copy
```
ID: cpp/bad-strncpy-size
Kind: problem
Severity: warning
Precision: medium
Tags: reliability correctness security external/cwe/cwe-676 external/cwe/cwe-119 external/cwe/cwe-251
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Memory%20Management/StrncpyFlippedArgs.ql)
The standard library function `strncpy` copies a source string to a destination buffer. The third argument defines the maximum number of characters to copy and should be less than or equal to the size of the destination buffer. Calls of the form `strncpy(dest, src, strlen(src))` or `strncpy(dest, src, sizeof(src))` incorrectly set the third argument to the size of the source buffer. Executing a call of this type may cause a buffer overflow. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.
## Recommendation
Check the highlighted function calls carefully, and ensure that the size parameter is derived from the size of the destination buffer, not the source buffer.
## Example
```cpp
strncpy(dest, src, sizeof(src)); //wrong: size of dest should be used
strncpy(dest, src, strlen(src)); //wrong: size of dest should be used
```
## References
* cplusplus.com: [strncpy](http://www.cplusplus.com/reference/clibrary/cstring/strncpy/).
* I. Gerg. *An Overview and Example of the Buffer-Overflow Exploit*. IANewsletter vol 7 no 4. 2005.
* M. Donaldson. *Inside the Buffer Overflow Attack: Mechanism, Method & Prevention*. SANS Institute InfoSec Reading Room. 2002.
* Common Weakness Enumeration: [CWE-676](https://cwe.mitre.org/data/definitions/676.html).
* Common Weakness Enumeration: [CWE-119](https://cwe.mitre.org/data/definitions/119.html).
* Common Weakness Enumeration: [CWE-251](https://cwe.mitre.org/data/definitions/251.html).

43
docs/language/query-help/cpp/SuspiciousAddWithSizeof.md
View File

@@ -1,43 +0,0 @@
# Suspicious add with sizeof
```
ID: cpp/suspicious-add-sizeof
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-468
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-468/SuspiciousAddWithSizeof.ql)
Pointer arithmetic in C and C++ is automatically scaled according to the size of the data type. For example, if the type of `p` is `T*` and `sizeof(T) == 4` then the expression `p+1` adds 4 bytes to `p`.
This query finds code of the form `p + k*sizeof(T)`. Such code is usually a mistake because there is no need to manually scale the offset by `sizeof(T)`.
## Recommendation
1. Whenever possible, use the array subscript operator rather than pointer arithmetic. For example, replace `*(p+k)` with `p[k]`.
1. Cast to the correct type before using pointer arithmetic. For example, if the type of `p` is `char*` but it really points to an array of type `double[]` then use the syntax `(double*)p + k` to get a pointer to the `k`'th element of the array.
## Example
```cpp
int example1(int i) {
int intArray[10] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
int *intPointer = intArray;
// BAD: the offset is already automatically scaled by sizeof(int),
// so this code will compute the wrong offset.
return *(intPointer + (i * sizeof(int)));
}
int example2(int i) {
int intArray[10] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
int *intPointer = intArray;
// GOOD: the offset is automatically scaled by sizeof(int).
return *(intPointer + i);
}
```
## References
* Common Weakness Enumeration: [CWE-468](https://cwe.mitre.org/data/definitions/468.html).

36
docs/language/query-help/cpp/SuspiciousCallToStrncat.md
View File

@@ -1,36 +0,0 @@
# Potentially unsafe call to strncat
```
ID: cpp/unsafe-strncat
Kind: problem
Severity: warning
Precision: medium
Tags: reliability correctness security external/cwe/cwe-676 external/cwe/cwe-119 external/cwe/cwe-251
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Memory%20Management/SuspiciousCallToStrncat.ql)
The standard library function `strncat` appends a source string to a target string. The third argument defines the maximum number of characters to append and should be less than or equal to the remaining space in the destination buffer. Calls of the form `strncat(dest, src, strlen(dest))` or `strncat(dest, src, sizeof(dest))` set the third argument to the entire size of the destination buffer. Executing a call of this type may cause a buffer overflow unless the buffer is known to be empty. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.
## Recommendation
Check the highlighted function calls carefully to ensure that no buffer overflow is possible. For a more robust solution, consider updating the function call to include the remaining space in the destination buffer.
## Example
```cpp
strncat(dest, src, strlen(dest)); //wrong: should use remaining size of dest
strncat(dest, src, sizeof(dest)); //wrong: should use remaining size of dest.
//Also fails if dest is a pointer and not an array.
```
## References
* cplusplus.com: [strncat](http://www.cplusplus.com/reference/clibrary/cstring/strncat/), [strncpy](http://www.cplusplus.com/reference/clibrary/cstring/strncpy/).
* I. Gerg, *An Overview and Example of the Buffer-Overflow Exploit*. IANewsletter vol 7 no 4, 2005.
* M. Donaldson, *Inside the Buffer Overflow Attack: Mechanism, Method & Prevention*. SANS Institute InfoSec Reading Room, 2002.
* Common Weakness Enumeration: [CWE-676](https://cwe.mitre.org/data/definitions/676.html).
* Common Weakness Enumeration: [CWE-119](https://cwe.mitre.org/data/definitions/119.html).
* Common Weakness Enumeration: [CWE-251](https://cwe.mitre.org/data/definitions/251.html).

32
docs/language/query-help/cpp/SuspiciousSizeof.md
View File

@@ -1,32 +0,0 @@
# Suspicious 'sizeof' use
```
ID: cpp/suspicious-sizeof
Kind: problem
Severity: warning
Precision: medium
Tags: reliability correctness security external/cwe/cwe-467
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Memory%20Management/SuspiciousSizeof.ql)
This rule finds expressions that take the size of a function parameter of array type. In C, function parameters of array type are treated as if they had the corresponding pointer type, so their size is always the size of the pointer type (typically either four or eight). In particular, one cannot determine the size of a memory buffer passed as a parameter in this way. Using the `sizeof` operator on pointer types will produce unexpected results if the developer intended to get the size of an array instead of the pointer.
## Recommendation
Modify the function to take an extra argument indicating the buffer size.
## Example
```cpp
void f(char s[]) {
int size = sizeof(s); //wrong: s is now a char*, not an array.
//sizeof(s) will evaluate to sizeof(char *)
}
```
## References
* Comp.lang.c, Frequently Asked Questions: [Question 6.3: So what is meant by the "equivalence of pointers and arrays" in C?](http://c-faq.com/aryptr/aryptrequiv.html).
* Common Weakness Enumeration: [CWE-467](https://cwe.mitre.org/data/definitions/467.html).

74
docs/language/query-help/cpp/TOCTOUFilesystemRace.md
View File

@@ -1,74 +0,0 @@
# Time-of-check time-of-use filesystem race condition
```
ID: cpp/toctou-race-condition
Kind: problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-367
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-367/TOCTOUFilesystemRace.ql)
Often it is necessary to check the state of a file before using it. These checks usually take a file name to be checked, and if the check returns positively, then the file is opened or otherwise operated upon.
However, in the time between the check and the operation, the underlying file referenced by the file name could be changed by an attacker, causing unexpected behavior.
## Recommendation
Wherever possible, use functions that operate on file descriptors rather than file names (for example, `fchmod` rather than `chmod`).
For access checks, you can temporarily change the UID and GID to that of the user whose permissions are being checked, and then perform the operation. This has the effect of "atomically" combining a permissions check with the operation.
If file-system locking tools are available on your platform, then locking the file before the check can prevent an unexpected update. However, note that on some platforms (for example, Unix) file-system locks are typically *advisory*, and so can be ignored by an attacker.
## Example
The following example shows a case where a file is opened and then, if the opening was successful, its permissions are changed with `chmod`. However, an attacker might change the target of the file name between the initial opening and the permissions change, potentially changing the permissions of a different file.
```c
char *file_name;
FILE *f_ptr;
/* Initialize file_name */
f_ptr = fopen(file_name, "w");
if (f_ptr == NULL) {
/* Handle error */
}
/* ... */
if (chmod(file_name, S_IRUSR) == -1) {
/* Handle error */
}
```
This can be avoided by using `fchmod` with the file descriptor that was received from opening the file. This ensures that the permissions change is applied to the very same file that was opened.
```c
char *file_name;
int fd;
/* Initialize file_name */
fd = open(
file_name,
O_WRONLY | O_CREAT | O_EXCL,
S_IRWXU
);
if (fd == -1) {
/* Handle error */
}
/* ... */
if (fchmod(fd, S_IRUSR) == -1) {
/* Handle error */
}
```
## References
* The CERT Oracle Secure Coding Standard for C: [ FIO01-C. Be careful using functions that use file names for identification ](https://www.securecoding.cert.org/confluence/display/c/FIO01-C.+Be+careful+using+functions+that+use+file+names+for+identification).
* Common Weakness Enumeration: [CWE-367](https://cwe.mitre.org/data/definitions/367.html).

41
docs/language/query-help/cpp/TaintedAllocationSize.md
View File

@@ -1,41 +0,0 @@
# Overflow in uncontrolled allocation size
```
ID: cpp/uncontrolled-allocation-size
Kind: path-problem
Severity: error
Precision: high
Tags: reliability security external/cwe/cwe-190
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql)
This code calculates an allocation size by multiplying a user input by a `sizeof` expression. Since the user input has no apparent guard on its magnitude, this multiplication can overflow. When an integer multiply overflows in C, the result can wrap around and be much smaller than intended. A later attempt to put data into the allocated buffer can then overflow.
## Recommendation
Guard all integer parameters that come from an external user. Implement a guard with the expected range for the parameter and make sure that the input value meets both the minimum and maximum requirements for this range. If the input value fails this guard then reject the request before proceeding further. If the input value passes the guard then subsequent calculations should not overflow.
## Example
```c
int factor = atoi(getenv("BRANCHING_FACTOR"));
// GOOD: Prevent overflow by checking the input
if (factor < 0 || factor > 1000) {
log("Factor out of range (%d)\n", factor);
return -1;
}
// This line can allocate too little memory if factor
// is very large.
char **root_node = (char **) malloc(factor * sizeof(char *));
```
This code shows one way to guard that an input value is within the expected range. If `factor` fails the guard, then an error is returned, and the value is not used as an argument to the subsequent call to `malloc`. Without this guard, the allocated buffer might be too small to hold the data intended for it.
## References
* The CERT Oracle Secure Coding Standard for C: [INT04-C. Enforce limits on integer values originating from tainted sources](https://www.securecoding.cert.org/confluence/display/c/INT04-C.+Enforce+limits+on+integer+values+originating+from+tainted+sources).
* Common Weakness Enumeration: [CWE-190](https://cwe.mitre.org/data/definitions/190.html).

45
docs/language/query-help/cpp/TaintedCondition.md
View File

@@ -1,45 +0,0 @@
# Untrusted input for a condition
```
ID: cpp/tainted-permissions-check
Kind: path-problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-807
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-807/TaintedCondition.ql)
This rule finds code where untrusted inputs are used in an `if` statement, and the body of that statement makes a security decision. This is an example of CWE-807 and makes the program vulnerable to attack. An attacker might be able to gain unauthorized access to the system by manipulating external inputs to the system.
## Recommendation
In most cases, you need to add or strengthen the checks made on the user-supplied data to ensure its integrity. The user-supplied data can then be used as a trusted input to the security decision. For example, instead of checking an HTTP cookie against a predictable fixed string, check a cookie against a randomly generated session key.
This rule may highlight a few conditions where user-supplied data has been checked and can be trusted. It is not always possible to determine if the checks applied to data are enough to ensure security.
## Example
The following example is included in CWE 807.
```c
struct hostent *hp;struct in_addr myaddr;
char* tHost = "trustme.example.com";
myaddr.s_addr=inet_addr(ip_addr_string);
hp = gethostbyaddr((char *) &myaddr, sizeof(struct in_addr), AF_INET);
if (hp && !strncmp(hp->h_name, tHost, sizeof(tHost))) {
trusted = true;
} else {
trusted = false;
}
```
In this example, the result of a reverse DNS query is compared against a fixed string. An attacker can return an incorrect reverse DNS entry for the requesting IP and thus gain the same access as a legitimate user from `trustme.example.com`.
To fix the problem in this example, you need to add an additional mechanism to test the user-supplied data. For example, numeric IP addresses could be used.
## References
* Common Weakness Enumeration: [CWE-807](https://cwe.mitre.org/data/definitions/807.html).

61
docs/language/query-help/cpp/TaintedPath.md
View File

@@ -1,61 +0,0 @@
# Uncontrolled data used in path expression
```
ID: cpp/path-injection
Kind: path-problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-022 external/cwe/cwe-023 external/cwe/cwe-036 external/cwe/cwe-073
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql)
Accessing paths controlled by users can allow an attacker to access unexpected resources. This can result in sensitive information being revealed or deleted, or an attacker being able to influence behavior by modifying unexpected files.
Paths that are naively constructed from data controlled by a user may contain unexpected special characters, such as "..". Such a path may potentially point to any directory on the filesystem.
## Recommendation
Validate user input before using it to construct a filepath. Ideally, follow these rules:
* Do not allow more than a single "." character.
* Do not allow directory separators such as "/" or "\" (depending on the filesystem).
* Do not rely on simply replacing problematic sequences such as "../". For example, after applying this filter to ".../...//" the resulting string would still be "../".
* Ideally use a whitelist of known good patterns.
## Example
In this example, a username and file are read from the arguments to main and then used to access a file in the user's home directory. However, a malicious user could enter a filename which contains special characters. For example, the string "../../etc/passwd" will result in the code reading the file located at "/home/[user]/../../etc/passwd", which is the system's password file. This could potentially allow them to access all the system's passwords.
```c
int main(int argc, char** argv) {
char *userAndFile = argv[2];
{
char fileBuffer[FILENAME_MAX] = "/home/";
char *fileName = fileBuffer;
size_t len = strlen(fileName);
strncat(fileName+len, userAndFile, FILENAME_MAX-len-1);
// BAD: a string from the user is used in a filename
fopen(fileName, "wb+");
}
{
char fileBuffer[FILENAME_MAX] = "/home/";
char *fileName = fileBuffer;
size_t len = strlen(fileName);
// GOOD: use a fixed file
char* fixed = "jim/file.txt";
strncat(fileName+len, fixed, FILENAME_MAX-len-1);
fopen(fileName, "wb+");
}
}
```
## References
* OWASP: [Path Traversal](https://www.owasp.org/index.php/Path_traversal).
* Common Weakness Enumeration: [CWE-22](https://cwe.mitre.org/data/definitions/22.html).
* Common Weakness Enumeration: [CWE-23](https://cwe.mitre.org/data/definitions/23.html).
* Common Weakness Enumeration: [CWE-36](https://cwe.mitre.org/data/definitions/36.html).
* Common Weakness Enumeration: [CWE-73](https://cwe.mitre.org/data/definitions/73.html).

42
docs/language/query-help/cpp/TooFewArguments.md
View File

@@ -1,42 +0,0 @@
# Call to function with fewer arguments than declared parameters
```
ID: cpp/too-few-arguments
Kind: problem
Severity: error
Precision: very-high
Tags: correctness maintainability security
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Underspecified%20Functions/TooFewArguments.ql)
A function is called with fewer arguments than there are parameters of the function.
This may indicate that an incorrect function is being called, or that the signature (parameter list) of the called function is not known to the author.
In C, function calls generally need to provide the same number of arguments as there are arguments to the function. (Variadic functions can accept additional arguments.) Providing fewer arguments than there are parameters is extremely dangerous, as the called function will nevertheless try to obtain the missing arguments' values, either from the stack or from machine registers. As a result, the function may behave unpredictably.
If the called function *modifies* a parameter corresponding to a missing argument, it may alter the state of the program upon its return. An attacker could use this to, for example, alter the control flow of the program to access forbidden resources.
## Recommendation
Call the function with the correct number of arguments.
## Example
```c
void one_argument();
void calls() {
one_argument(1); // GOOD: `one_argument` will accept and use the argument
one_argument(); // BAD: `one_argument` will receive an undefined value
}
void one_argument(int x);
```
## References
* SEI CERT C Coding Standard: [ DCL20-C. Explicitly specify void when a function accepts no arguments ](https://wiki.sei.cmu.edu/confluence/display/c/DCL20-C.+Explicitly+specify+void+when+a+function+accepts+no+arguments)

42
docs/language/query-help/cpp/UnboundedWrite.md
View File

@@ -1,42 +0,0 @@
# Unbounded write
```
ID: cpp/unbounded-write
Kind: path-problem
Severity: error
Precision: medium
Tags: reliability security external/cwe/cwe-120 external/cwe/cwe-787 external/cwe/cwe-805
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql)
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.
## Recommendation
Always control the length of buffer copy and buffer write operations. `strncpy` should be used over `strcpy`, `snprintf` over `sprintf` etc. In general 'n-variant' functions should be preferred.
## Example
```c
void congratulateUser(const char *userName)
{
char buffer[80];
// BAD: this could overflow the buffer if the UserName is long
sprintf(buffer, "Congratulations, %s!", userName);
MessageBox(hWnd, buffer, "New Message", MB_OK);
}
```
In this example, the call to `sprintf` may overflow `buffer`. This occurs if the argument `userName` is very long, such that the resulting string is more than the 80 characters allowed.
To fix the problem the call to `sprintf` should be replaced with `snprintf`, specifying a maximum length of 80 characters.
## References
* CERT C++ Coding Standard: [STR50-CPP. Guarantee that storage for strings has sufficient space for character data and the null terminator](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).
* Common Weakness Enumeration: [CWE-120](https://cwe.mitre.org/data/definitions/120.html).
* Common Weakness Enumeration: [CWE-787](https://cwe.mitre.org/data/definitions/787.html).
* Common Weakness Enumeration: [CWE-805](https://cwe.mitre.org/data/definitions/805.html).

45
docs/language/query-help/cpp/UncontrolledFormatString.md
View File

@@ -1,45 +0,0 @@
# Uncontrolled format string
```
ID: cpp/tainted-format-string
Kind: path-problem
Severity: warning
Precision: high
Tags: reliability security external/cwe/cwe-134
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatString.ql)
The program uses input from the user as a format string for `printf` style functions. This can lead to buffer overflows or data representation problems. An attacker can exploit this weakness to crash the program, disclose information or even execute arbitrary code.
The results of this rule do not include inputs from the user that are transferred through global variables. Those can be found in the related rule "Uncontrolled format string (through global variable)".
## Recommendation
Use constant expressions as the format strings. If you need to print a value from the user, use `printf("%s", value_from_user)`.
## Example
```c
#include <stdio.h>
void printWrapper(char *str) {
printf(str);
}
int main(int argc, char **argv) {
// This should be avoided
printf(argv[1]);
// This should be avoided too, because it has the same effect
printWrapper(argv[1]);
// This is fine
printf("%s", argv[1]);
}
```
## References
* CERT C Coding Standard: [FIO30-C. Exclude user input from format strings](https://www.securecoding.cert.org/confluence/display/c/FIO30-C.+Exclude+user+input+from+format+strings).
* Common Weakness Enumeration: [CWE-134](https://cwe.mitre.org/data/definitions/134.html).

56
docs/language/query-help/cpp/UncontrolledFormatStringThroughGlobalVar.md
View File

@@ -1,56 +0,0 @@
# Uncontrolled format string (through global variable)
```
ID: cpp/tainted-format-string-through-global
Kind: path-problem
Severity: warning
Precision: high
Tags: reliability security external/cwe/cwe-134
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatStringThroughGlobalVar.ql)
The program uses input from the user, propagated via a global variable, as a format string for `printf` style functions. This can lead to buffer overflows or data representation problems. An attacker can exploit this weakness to crash the program, disclose information or even execute arbitrary code.
This rule only identifies inputs from the user that are transferred through global variables before being used in `printf` style functions. Analyzing the flow of data through global variables is more prone to errors and so this rule may identify some examples of code where the input is not really from the user. For example, when a global variable is set in two places, one that comes from the user and one that does not. In this case we would mark all usages of the global variable as input from the user, but the input from the user may always came after the call to the `printf` style functions.
The results of this rule should be considered alongside the related rule "Uncontrolled format string" which tracks the flow of the values input by a user, excluding global variables, until the values are used as the format argument for a `printf` like function call.
## Recommendation
Use constant expressions as the format strings. If you need to print a value from the user, use `printf("%s", value_from_user)`.
## Example
```c
#include <stdio.h>
char *copy;
void copyArgv(char **argv) {
copy = argv[1];
}
void printWrapper(char *str) {
printf(str);
}
int main(int argc, char **argv) {
copyArgv(argv);
// This should be avoided
printf(copy);
// This should be avoided too, because it has the same effect
printWrapper(copy);
// This is fine
printf("%s", copy);
}
```
## References
* CERT C Coding Standard: [FIO30-C. Exclude user input from format strings](https://www.securecoding.cert.org/confluence/display/c/FIO30-C.+Exclude+user+input+from+format+strings).
* Common Weakness Enumeration: [CWE-134](https://cwe.mitre.org/data/definitions/134.html).

46
docs/language/query-help/cpp/UncontrolledProcessOperation.md
View File

@@ -1,46 +0,0 @@
# Uncontrolled process operation
```
ID: cpp/uncontrolled-process-operation
Kind: path-problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-114
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-114/UncontrolledProcessOperation.ql)
The code passes user input directly to `system`, `dlopen`, `LoadLibrary` or some other process or library routine. As a result, the user can cause execution of arbitrary code.
## Recommendation
If possible, use hard-coded string literals for the command to run or library to load. Instead of passing the user input directly to the process or library function, examine the user input and then choose among hard-coded string literals.
If the applicable libraries or commands cannot be determined at compile time, then add code to verify that the user-input string is safe before using it.
## Example
```c
int main(int argc, char** argv) {
char *lib = argv[2];
// BAD: the user can cause arbitrary code to be loaded
void* handle = dlopen(lib, RTLD_LAZY);
// GOOD: only hard-coded libraries can be loaded
void* handle2;
if (!strcmp(lib, "inmem")) {
handle2 = dlopen("/usr/share/dbwrap/inmem", RTLD_LAZY);
} else if (!strcmp(lib, "mysql")) {
handle2 = dlopen("/usr/share/dbwrap/mysql", RTLD_LAZY);
} else {
die("Invalid library specified\n");
}
}
```
## References
* Common Weakness Enumeration: [CWE-114](https://cwe.mitre.org/data/definitions/114.html).

61
docs/language/query-help/cpp/UninitializedLocal.md
View File

@@ -1,61 +0,0 @@
# Potentially uninitialized local variable
```
ID: cpp/uninitialized-local
Kind: problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-665 external/cwe/cwe-457
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Memory%20Management/UninitializedLocal.ql)
A local non-static variable of a non-class type has an undefined value before it is initialized. For example, it is incorrect to rely on an uninitialized integer to have the value `0`.
## Recommendation
Review the code and consider whether the variable should have an initializer or whether some path through the program lacks an assignment to the variable.
## Example
The function `absWrong` does not initialize the variable `j` in the case where `i = 0`. Functions `absCorrect1` and `absCorrect2` remedy this deficiency by adding an initializer and adding an assignment to one of the paths through the program, respectively.
```cpp
int absWrong(int i) {
int j;
if (i > 0) {
j = i;
} else if (i < 0) {
j = -i;
}
return j; // wrong: j may not be initialized before use
}
int absCorrect1(int i) {
int j = 0;
if (i > 0) {
j = i;
} else if (i < 0) {
j = -i;
}
return j; // correct: j always initialized before use
}
int absCorrect2(int i) {
int j;
if (i > 0) {
j = i;
} else if (i < 0) {
j = -i;
} else {
j = 0;
}
return j; // correct: j always initialized before use
}
```
## References
* ISO/IEC 9899:2011: [Programming languages - C (Section 6.3.2.1)](https://www.iso.org/standard/57853.html).
* Common Weakness Enumeration: [CWE-665](https://cwe.mitre.org/data/definitions/665.html).
* Common Weakness Enumeration: [CWE-457](https://cwe.mitre.org/data/definitions/457.html).

53
docs/language/query-help/cpp/UnsafeCreateProcessCall.md
View File

@@ -1,53 +0,0 @@
# NULL application name with an unquoted path in call to CreateProcess
```
ID: cpp/unsafe-create-process-call
Kind: problem
Severity: error
Precision: medium
Tags: security external/cwe/cwe-428 external/microsoft/C6277
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-428/UnsafeCreateProcessCall.ql)
This query indicates that there is a call to a function of the `CreateProcess*` family of functions, which introduces a security vulnerability.
## Recommendation
Do not use `NULL` for the `lpApplicationName` argument to the `CreateProcess*` function.
If you pass `NULL` for `lpApplicationName`, use quotation marks around the executable path in `lpCommandLine`.
## Example
In the following example, `CreateProcessW` is called with a `NULL` value for `lpApplicationName`, and the value for `lpCommandLine` that represent the application path is not quoted and has spaces in it.
If an attacker has access to the file system, they can elevate privileges by creating a file such as `C:\Program.exe` that will be executed instead of the intended application.
```cpp
STARTUPINFOW si;
PROCESS_INFORMATION pi;
// ...
CreateProcessW( // BUG
NULL, // lpApplicationName
(LPWSTR)L"C:\\Program Files\\MyApp", // lpCommandLine
NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi);
// ...
```
To fix this issue, specify a valid string for `lpApplicationName`, or quote the path for `lpCommandLine`. For example:
`(LPWSTR)L"\"C:\\Program Files\\MyApp\"", // lpCommandLine`
## References
* [CreateProcessA function (Microsoft documentation).](https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessa)
* [CreateProcessW function (Microsoft documentation).](https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessw)
* [CreateProcessAsUserA function (Microsoft documentation).](https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessasusera)
* [CreateProcessAsUserW function (Microsoft documentation).](https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessasuserw)
* [CreateProcessWithLogonW function (Microsoft documentation).](https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-createprocesswithlogonw)
* [CreateProcessWithTokenW function (Microsoft documentation).](https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-createprocesswithtokenw)
* Common Weakness Enumeration: [CWE-428](https://cwe.mitre.org/data/definitions/428.html).

52
docs/language/query-help/cpp/UnsafeDaclSecurityDescriptor.md
View File

@@ -1,52 +0,0 @@
# Setting a DACL to NULL in a SECURITY_DESCRIPTOR
```
ID: cpp/unsafe-dacl-security-descriptor
Kind: problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-732 external/microsoft/C6248
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-732/UnsafeDaclSecurityDescriptor.ql)
This query indicates that a call is setting the DACL field in a `SECURITY_DESCRIPTOR` to null.
When using `SetSecurityDescriptorDacl` to set a discretionary access control (DACL), setting the `bDaclPresent` argument to `TRUE` indicates the prescence of a DACL in the security description in the argument `pDacl`.
When the `pDacl` parameter does not point to a DACL (i.e. it is `NULL`) and the `bDaclPresent` flag is `TRUE`, a `NULL DACL` is specified.
A `NULL DACL` grants full access to any user who requests it; normal security checking is not performed with respect to the object.
## Recommendation
You should not use a `NULL DACL` with an object because any user can change the DACL and owner of the security descriptor.
## Example
In the following example, the call to `SetSecurityDescriptorDacl` is setting an unsafe DACL (`NULL DACL`) to the security descriptor.
```cpp
SECURITY_DESCRIPTOR pSD;
SECURITY_ATTRIBUTES SA;
if (!InitializeSecurityDescriptor(&pSD, SECURITY_DESCRIPTOR_REVISION))
{
// error handling
}
if (!SetSecurityDescriptorDacl(&pSD,
TRUE, // bDaclPresent - this value indicates the presence of a DACL in the security descriptor
NULL, // pDacl - the pDacl parameter does not point to a DACL. All access will be allowed
FALSE))
{
// error handling
}
```
To fix this issue, `pDacl` argument should be a pointer to an `ACL` structure that specifies the DACL for the security descriptor.
## References
* [SetSecurityDescriptorDacl function (Microsoft documentation).](https://docs.microsoft.com/en-us/windows/desktop/api/securitybaseapi/nf-securitybaseapi-setsecuritydescriptordacl)
* Common Weakness Enumeration: [CWE-732](https://cwe.mitre.org/data/definitions/732.html).

43
docs/language/query-help/cpp/UnsafeUseOfStrcat.md
View File

@@ -1,43 +0,0 @@
# Potentially unsafe use of strcat
```
ID: cpp/unsafe-strcat
Kind: problem
Severity: warning
Precision: medium
Tags: reliability correctness security external/cwe/cwe-676 external/cwe/cwe-120 external/cwe/cwe-251
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Memory%20Management/UnsafeUseOfStrcat.ql)
The standard library function `strcat` appends a source string to a target string. If you do not check the size of the source string then you cannot guarantee that appending the data to the target string will not cause a buffer overflow. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.
## Recommendation
Check the highlighted function calls carefully to ensure that no buffer overflow is possible. For a more robust solution, consider adding explicit range checks or using the `strncat` function instead.
## Example
```cpp
void f(char *s) {
char buf[80];
strcpy(buf, "s: ");
strcat(buf, s); // wrong: buffer not checked before strcat
}
void g(char *s) {
char buf[80];
strcpy(buf, "s: ");
if(strlen(s) < 77)
strcat(buf, s); // correct: buffer size checked before strcat
}
```
## References
* I. Gerg, *An Overview and Example of the Buffer-Overflow Exploit*. IANewsletter vol 7, no 4, 2005.
* M. Donaldson, *Inside the Buffer Overflow Attack: Mechanism, Method & Prevention*. SANS Institute InfoSec Reading Room. 2002.
* Common Weakness Enumeration: [CWE-676](https://cwe.mitre.org/data/definitions/676.html).
* Common Weakness Enumeration: [CWE-120](https://cwe.mitre.org/data/definitions/120.html).
* Common Weakness Enumeration: [CWE-251](https://cwe.mitre.org/data/definitions/251.html).

59
docs/language/query-help/cpp/UnterminatedVarargsCall.md
View File

@@ -1,59 +0,0 @@
# Unterminated variadic call
```
ID: cpp/unterminated-variadic-call
Kind: problem
Severity: warning
Precision: medium
Tags: reliability security external/cwe/cwe-121
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-121/UnterminatedVarargsCall.ql)
The program calls a function that expects the variable argument list to be terminated with a sentinel value (typically NULL, 0 or -1). In this case, the sentinel value has been omitted as a final argument. This defect may result in incorrect behavior of the function and unintended stack memory access, leading to incorrect program results, instability, and even vulnerability to buffer overflow style attacks.
## Recommendation
Each description of a defect highlighted by this rule includes a suggested value for the terminator. Check that this value is correct, then add it to the end of the call.
## Example
```cpp
#include <stdarg.h>
void pushStrings(char *firstString, ...)
{
va_list args;
char *arg;
va_start(args, firstString);
// process inputs, beginning with firstString, ending when NULL is reached
arg = firstString;
while (arg != NULL)
{
// push the string
pushString(arg);
// move on to the next input
arg = va_arg(args, char *);
}
va_end(args);
}
void badFunction()
{
pushStrings("hello", "world", NULL); // OK
pushStrings("apple", "pear", "banana", NULL); // OK
pushStrings("car", "bus", "train"); // BAD, not terminated with the expected NULL
}
```
In this example, the third call to `pushStrings` is not correctly terminated. This call should be updated to include `NULL` as the fourth and final argument to this call.
## References
* Common Weakness Enumeration: [CWE-121](https://cwe.mitre.org/data/definitions/121.html).

41
docs/language/query-help/cpp/WcharCharConversion.md
View File

@@ -1,41 +0,0 @@
# Cast from char* to wchar_t*
```
ID: cpp/incorrect-string-type-conversion
Kind: problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-704 external/microsoft/c/c6276
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.ql)
This rule indicates a potentially incorrect cast from an byte string (`char *`) to a wide-character string (`wchar_t *`).
This cast might yield strings that are not correctly terminated; including potential buffer overruns when using such strings with some dangerous APIs.
## Recommendation
Do not explicitly cast byte strings to wide-character strings.
For string literals, prepend the literal string with the letter "L" to indicate that the string is a wide-character string (`wchar_t *`).
For converting a byte literal to a wide-character string literal, you would need to use the appropriate conversion function for the platform you are using. Please see the references section for options according to your platform.
## Example
In the following example, an byte string literal (`"a"`) is cast to a wide-character string.
```cpp
wchar_t* pSrc;
pSrc = (wchar_t*)"a"; // casting a byte-string literal "a" to a wide-character string
```
To fix this issue, prepend the literal with the letter "L" (`L"a"`) to define it as a wide-character string.
## References
* General resources: [std::mbstowcs](https://en.cppreference.com/w/cpp/string/multibyte/mbstowcs)
* Microsoft specific resources: [Security Considerations: International Features](https://docs.microsoft.com/en-us/windows/desktop/Intl/security-considerations--international-features)
* Common Weakness Enumeration: [CWE-704](https://cwe.mitre.org/data/definitions/704.html).

36
docs/language/query-help/cpp/WrongNumberOfFormatArguments.md
View File

@@ -1,36 +0,0 @@
# Too few arguments to formatting function
```
ID: cpp/wrong-number-format-arguments
Kind: problem
Severity: error
Precision: high
Tags: reliability correctness security external/cwe/cwe-685
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Format/WrongNumberOfFormatArguments.ql)
Each call to the `printf` function, or a related function, should include the number of arguments defined by the format. Passing the function more arguments than required is harmless (although it may be indicative of other defects). However, passing the function fewer arguments than are defined by the format can be a security vulnerability since the function will process the next item on the stack as the missing arguments.
This might lead to an information leak if a sensitive value from the stack is printed. It might cause a crash if a value on the stack is interpreted as a pointer and leads to accessing unmapped memory. Finally, it may lead to a follow-on vulnerability if an attacker can use this problem to cause the output string to be too long or have unexpected contents.
## Recommendation
Review the format and arguments expected by the highlighted function calls. Update either the format or the arguments so that the expected number of arguments are passed to the function.
## Example
```cpp
int main() {
printf("%d, %s\n", 42); // Will crash or print garbage
return 0;
}
```
## References
* CERT C Coding Standard: [FIO30-C. Exclude user input from format strings](https://www.securecoding.cert.org/confluence/display/c/FIO30-C.+Exclude+user+input+from+format+strings).
* cplusplus.com: [C++ Functions](http://www.tutorialspoint.com/cplusplus/cpp_functions.htm).
* Microsoft C Runtime Library Reference: [printf, wprintf](https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/printf-printf-l-wprintf-wprintf-l).
* Common Weakness Enumeration: [CWE-685](https://cwe.mitre.org/data/definitions/685.html).

34
docs/language/query-help/cpp/WrongTypeFormatArguments.md
View File

@@ -1,34 +0,0 @@
# Wrong type of arguments to formatting function
```
ID: cpp/wrong-type-format-argument
Kind: problem
Severity: error
Precision: high
Tags: reliability correctness security external/cwe/cwe-686
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/cpp/ql/src/Likely%20Bugs/Format/WrongTypeFormatArguments.ql)
Each call to the `printf` function or a related function should include the type and sequence of arguments defined by the format. If the function is passed arguments of a different type or in a different sequence then the arguments are reinterpreted to fit the type and sequence expected, resulting in unpredictable behavior.
## Recommendation
Review the format and arguments expected by the highlighted function calls. Update either the format or the arguments so that the expected type and sequence of arguments are passed to the function.
## Example
```cpp
int main() {
printf("%s\n", 42); //printf will treat 42 as a char*, will most likely segfault
return 0;
}
```
## References
* CERT C Coding Standard: [FIO30-C. Exclude user input from format strings](https://www.securecoding.cert.org/confluence/display/c/FIO30-C.+Exclude+user+input+from+format+strings).
* cplusplus.com: [C++ Functions](http://www.tutorialspoint.com/cplusplus/cpp_functions.htm).
* MSDN Alphabetical Function Reference: [printf, wprintf](http://msdn.microsoft.com/en-us/library/wc7014hz%28VS.71%29.aspx).
* Common Weakness Enumeration: [CWE-686](https://cwe.mitre.org/data/definitions/686.html).

56
docs/language/query-help/csharp/ASPNetDebug.md
View File

@@ -1,56 +0,0 @@
# Creating an ASP.NET debug binary may reveal sensitive information
```
ID: cs/web/debug-binary
Kind: problem
Severity: warning
Precision: very-high
Tags: security maintainability frameworks/asp.net external/cwe/cwe-11
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-011/ASPNetDebug.ql)
ASP.NET applications that deploy a 'debug' build to production can reveal debugging information to end users. This debugging information can aid a malicious user in attacking the system. The use of the debugging flag may also impair performance, increasing execution time and memory usage.
## Recommendation
Remove the 'debug' flag from the `Web.config` file if this configuration is likely to be used in production.
## Example
The following example shows the 'debug' flag set to true in a `Web.config` file for ASP.NET:
```none
<?xml version="1.0" encoding="utf-8" ?>
<configuration>
<system.web>
<compilation
defaultLanguage="c#"
debug="true"
/>
...
</system.web>
</configuration>
```
This will produce a 'debug' build that may be exploited by an end user.
To fix this problem, the 'debug' flag should be set to `false`, or removed completely:
```none
<?xml version="1.0" encoding="utf-8" ?>
<configuration>
<system.web>
<compilation
defaultLanguage="c#"
/>
...
</system.web>
</configuration>
```
## References
* MSDN: [Why debug=false in ASP.NET applications in production environment](https://blogs.msdn.microsoft.com/prashant_upadhyay/2011/07/14/why-debugfalse-in-asp-net-applications-in-production-environment/).
* MSDN: [How to: Enable Debugging for ASP.NET Applications](https://msdn.microsoft.com/en-us/library/e8z01xdh.aspx).
* Common Weakness Enumeration: [CWE-11](https://cwe.mitre.org/data/definitions/11.html).

48
docs/language/query-help/csharp/ASPNetDirectoryListing.md
View File

@@ -1,48 +0,0 @@
# ASP.NET config file enables directory browsing
```
ID: cs/web/directory-browse-enabled
Kind: problem
Severity: warning
Precision: very-high
Tags: security external/cwe/cwe-548
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-548/ASPNetDirectoryListing.ql)
ASP.NET applications that enable directory browsing can leak sensitive information to an attacker. The precise nature of the vulnerability depends on which files are listed and accessible.
## Recommendation
If this configuration may be used in production, remove the `directoryBrowse` element from the `Web.config` file or set the value to false.
## Example
The following example shows the `directoryBrowse` `enable` attribute set to true in a `Web.config` file for ASP.NET:
```none
<?xml version="1.0" encoding="utf-8" ?>
<configuration>
<system.webServer>
<directoryBrowse enable="true"/>
...
</system.web>
</configuration>
```
To fix this problem, the `enable` attribute should be set to `false`, or the `directoryBrowse` element should be removed completely:
```none
<?xml version="1.0" encoding="utf-8" ?>
<configuration>
<system.webServer>
<directoryBrowse enable="false"/>
...
</system.web>
</configuration>
```
## References
* MSDN: [directoryBrowse element](https://msdn.microsoft.com/en-us/library/ms691327(v=vs.90).aspx).
* Common Weakness Enumeration: [CWE-548](https://cwe.mitre.org/data/definitions/548.html).

52
docs/language/query-help/csharp/AbandonSession.md
View File

@@ -1,52 +0,0 @@
# Failure to abandon session
```
ID: cs/session-reuse
Kind: problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-384
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-384/AbandonSession.ql)
Reusing a session could allow an attacker to gain unauthorized access to another account. Always ensure that, when a user logs in or out, the current session is abandoned so that a new session may be started.
## Recommendation
Always call `HttpSessionState.Abandon()` to ensure that the previous session is not used by the new user.
## Example
The following example shows the previous session being used after authentication. This would allow a previous user to use the new user's account.
```csharp
public void Login(HttpContext ctx, string username, string password)
{
if (FormsAuthentication.Authenticate(username, password)
{
// BAD: Reusing the previous session
ctx.Session["Mode"] = GetModeForUser(username);
}
}
```
This code example solves the problem by not reusing the session, and instead calling `Abandon()` to ensure that the session is not reused.
```csharp
public void Login(HttpContext ctx, string username, string password)
{
if (FormsAuthentication.Authenticate(username, password)
{
// GOOD: Abandon the session first.
ctx.Session.Abandon();
}
}
```
## References
* MSDN: [ASP.NET Session State Overview](https://msdn.microsoft.com/en-us/library/ms178581.aspx), [HttpSessionState.Abandon Method ()](https://msdn.microsoft.com/en-us/library/system.web.sessionstate.httpsessionstate.abandon(v=vs.110).aspx).
* Common Weakness Enumeration: [CWE-384](https://cwe.mitre.org/data/definitions/384.html).

71
docs/language/query-help/csharp/AssemblyPathInjection.md
View File

@@ -1,71 +0,0 @@
# Assembly path injection
```
ID: cs/assembly-path-injection
Kind: problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-114
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-114/AssemblyPathInjection.ql)
C# supports runtime loading of assemblies by path through the use of the `System.Reflection.Assembly` API. If an external user can influence the path used to load an assembly, then the application can potentially be tricked into loading an assembly which was not intended to be loaded, and executing arbitrary code.
## Recommendation
Avoid loading assemblies based on user provided input. If this is not possible, ensure that the path is validated before being used with `Assembly`. For example, compare the provided input against a whitelist of known safe assemblies, or confirm that the path is restricted to a single directory which only contains safe assemblies.
## Example
In this example, user input is provided describing the path to an assembly, which is loaded without validation. This is problematic because it allows the user to load any assembly installed on the system, and is particularly problematic if an attacker can upload a custom DLL elsewhere on the system.
```csharp
using System;
using System.Web;
using System.Reflection;
public class AssemblyPathInjectionHandler : IHttpHandler {
public void ProcessRequest(HttpContext ctx) {
string assemblyPath = ctx.Request.QueryString["assemblyPath"];
// BAD: Load assembly based on user input
var badAssembly = Assembly.LoadFile(assemblyPath);
// Method called on loaded assembly. If the user can control the loaded assembly, then this
// could result in a remote code execution vulnerability
MethodInfo m = badAssembly.GetType("Config").GetMethod("GetCustomPath");
Object customPath = m.Invoke(null, null);
// ...
}
}
```
In the corrected version, user input is validated against one of two options, and the assembly is only loaded if the user input matches one of those options.
```csharp
using System;
using System.Web;
using System.Reflection;
public class AssemblyPathInjectionHandler : IHttpHandler {
public void ProcessRequest(HttpContext ctx) {
string configType = ctx.Request.QueryString["configType"];
if (configType.equals("configType1") || configType.equals("configType2")) {
// GOOD: Loaded assembly is one of the two known safe options
var safeAssembly = Assembly.LoadFile(@"C:\SafeLibraries\" + configType + ".dll");
// Code execution is limited to one of two known and vetted assemblies
MethodInfo m = safeAssembly.GetType("Config").GetMethod("GetCustomPath");
Object customPath = m.Invoke(null, null);
// ...
}
}
}
```
## References
* Microsoft: [System.Reflection.Assembly](https://docs.microsoft.com/en-us/dotnet/api/system.reflection.assembly?view=netframework-4.8).
* Common Weakness Enumeration: [CWE-114](https://cwe.mitre.org/data/definitions/114.html).

64
docs/language/query-help/csharp/CleartextStorage.md
View File

@@ -1,64 +0,0 @@
# Clear text storage of sensitive information
```
ID: cs/cleartext-storage-of-sensitive-information
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-312 external/cwe/cwe-315 external/cwe/cwe-359
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-312/CleartextStorage.ql)
Sensitive information that is stored unencrypted is accessible to an attacker who gains access to the storage. This is particularly important for cookies, which are stored on the machine of the end-user.
## Recommendation
Ensure that sensitive information is always encrypted before being stored. For ASP.NET applications, the `System.Web.Security.MachineKey` class may be used to encode sensitive information.
If possible, avoid placing sensitive information in cookies all together. Instead, prefer storing a key in the cookie that can be used to lookup the sensitive information.
In general, decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.
## Example
The following example shows two ways of storing user credentials in a cookie. In the 'BAD' case, the credentials are simply stored in cleartext. In the 'GOOD' case, the credentials are protected before storing them, using `MachineKey.Protect`, wrapped in a utility method.
```csharp
using System.Text;
using System.Web;
using System.Web.Security;
public class CleartextStorageHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string accountName = ctx.Request.QueryString["AccountName"];
// BAD: Setting a cookie value with cleartext sensitive data.
ctx.Response.Cookies["AccountName"].Value = accountName;
// GOOD: Encoding the value before setting it.
ctx.Response.Cookies["AccountName"].Value = Protect(accountName, "Account name");
}
/// <summary>
/// Protect the cleartext value, using the given type.
/// </summary>
/// <value>
/// The protected value, which is no longer cleartext.
/// </value>
public string Protect(string value, string type)
{
return Encoding.UTF8.GetString(MachineKey.Protect(Encoding.UTF8.GetBytes(value), type));
}
}
```
## References
* M. Dowd, J. McDonald and J. Schuhm, *The Art of Software Security Assessment*, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
* M. Howard and D. LeBlanc, *Writing Secure Code*, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
* Common Weakness Enumeration: [CWE-312](https://cwe.mitre.org/data/definitions/312.html).
* Common Weakness Enumeration: [CWE-315](https://cwe.mitre.org/data/definitions/315.html).
* Common Weakness Enumeration: [CWE-359](https://cwe.mitre.org/data/definitions/359.html).

75
docs/language/query-help/csharp/CodeInjection.md
View File

@@ -1,75 +0,0 @@
# Improper control of generation of code
```
ID: cs/code-injection
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-094 external/cwe/cwe-095 external/cwe/cwe-096
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-094/CodeInjection.ql)
If the application dynamically compiles and runs source code constructed from user input, a malicious user may be able to run arbitrary code.
## Recommendation
It is good practice not to generate, compile and run source code constructed from untrusted user input. If code must be dynamically generated using user input, the user input should be validated to prevent arbitrary code from appearing in the input. For example, a whitelist may be used to ensure that the input is limited to an acceptable range of values.
## Example
In the following example, the HttpHandler accepts remote user input which is C# source code for calculating tax. It compiles and runs this code, returning the output. However, the user provided source code is entirely unvalidated, and therefore allows arbitrary code execution.
If possible, the dynamic compilation should be removed all together, and replaced with a fixed set of tax calculation algorithms. If this is not sufficiently powerful, an interpreter could be provided for a safe, restricted language.
```csharp
using Microsoft.CSharp;
using System;
using System.CodeDom.Compiler;
using System.Reflection;
using System.Web;
public class CodeInjectionHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
// Code for calculating tax is provided as unvalidated user input
string taxFormula = ctx.Request.QueryString["tax_formula"];
// Used to create C#
StringBuilder sourceCode = new StringBuilder("");
sourceCode.Append("public class TaxCalc {\n");
sourceCode.Append("\tpublic int CalculateTax(int value){\n");
sourceCode.Append("\t\treturn " + taxFormula + "; \n");
sourceCode.Append("\t}\n");
sourceCode.Append("}\n");
// BAD: This compiles the sourceCode, containing unvalidated user input
CSharpCodeProvider c = new CSharpCodeProvider();
ICodeCompiler icc = c.CreateCompiler();
CompilerParameters cp = new CompilerParameters();
CompilerResults cr = icc.CompileAssemblyFromSource(cp, sourceCode.ToString());
// Compiled input is loaded, and an instance of the class is constructed
System.Reflection.Assembly a = cr.CompiledAssembly;
object taxCalc = a.CreateInstance("TaxCalc");
// Unsafe code is executed
Type taxCalcType = o.GetType();
MethodInfo mi = type.GetMethod("CalculateTax");
int value = int.Parse(ctx.Request.QueryString["value"]);
int s = (int)mi.Invoke(o, new object[] { value });
// Result is returned to the user
ctx.Response.Write("Tax value is: " + s);
}
}
```
## References
* Wikipedia: [Code Injection](https://en.wikipedia.org/wiki/Code_injection).
* OWASP: [Code Injection](https://www.owasp.org/index.php/Code_Injection).
* Common Weakness Enumeration: [CWE-94](https://cwe.mitre.org/data/definitions/94.html).
* Common Weakness Enumeration: [CWE-95](https://cwe.mitre.org/data/definitions/95.html).
* Common Weakness Enumeration: [CWE-96](https://cwe.mitre.org/data/definitions/96.html).

45
docs/language/query-help/csharp/CommandInjection.md
View File

@@ -1,45 +0,0 @@
# Uncontrolled command line
```
ID: cs/command-line-injection
Kind: path-problem
Severity: error
Precision: high
Tags: correctness security external/cwe/cwe-078 external/cwe/cwe-088
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-078/CommandInjection.ql)
Code that passes user input directly to `System.Diagnostic.Process.Start`, or some other library routine that executes a command, allows the user to execute malicious code.
## Recommendation
If possible, use hard-coded string literals to specify the command to run or library to load. Instead of passing the user input directly to the process or library function, examine the user input and then choose among hard-coded string literals.
If the applicable libraries or commands cannot be determined at compile time, then add code to verify that the user input string is safe before using it.
## Example
The following example shows code that takes a shell script that can be changed maliciously by a user, and passes it straight to `System.Diagnostic.Process.Start` without examining it first.
```csharp
using System;
using System.Web;
using System.Diagnostics;
public class CommandInjectionHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string param = ctx.Request.QueryString["param"];
Process.Start("process.exe", "/c " + param);
}
}
```
## References
* OWASP: [Command Injection](https://www.owasp.org/index.php/Command_Injection).
* Common Weakness Enumeration: [CWE-78](https://cwe.mitre.org/data/definitions/78.html).
* Common Weakness Enumeration: [CWE-88](https://cwe.mitre.org/data/definitions/88.html).

54
docs/language/query-help/csharp/ConditionalBypass.md
View File

@@ -1,54 +0,0 @@
# User-controlled bypass of sensitive method
```
ID: cs/user-controlled-bypass
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-807 external/cwe/cwe-247 external/cwe/cwe-350
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-807/ConditionalBypass.ql)
Many C# constructs enable code statements to be executed conditionally, for example, `if` statements and `for` statements. If the statements contain important authentication or login code, and user-controlled data determines whether or not the code is executed, an attacker may be able to bypass security systems.
## Recommendation
Never decide whether to authenticate a user based on data that may be controlled by that user. If necessary, ensure that the data is validated extensively when it is input before any authentication checks are performed.
It is still possible to have a system that "remembers" users, thus not requiring the user to login on every interaction. For example, personalization settings can be applied without authentication because this is not sensitive information. However, users should be allowed to take sensitive actions only when they have been fully authenticated.
## Example
This example shows two ways of deciding whether to authenticate a user. The first way shows a decision that is based on the value of a cookie. Cookies can be easily controlled by the user, and so this allows a user to become authenticated without providing valid credentials. The second, more secure way shows a decision that is based on looking up the user in a security database.
```csharp
public boolean doLogin(HttpCookie adminCookie, String user, String password)
{
// BAD: login is executed only if the value of 'adminCookie' is 'false',
// but 'adminCookie' is controlled by the user
if (adminCookie.Value == "false")
return login(user, password);
return true;
}
public boolean doLogin(HttpCookie adminCookie, String user, String password)
{
// GOOD: use server-side information based on the credentials to decide
// whether user has privileges
bool isAdmin = queryDbForAdminStatus(user, password);
if (!isAdmin)
return login(user, password);
return true;
}
```
## References
* Common Weakness Enumeration: [CWE-807](https://cwe.mitre.org/data/definitions/807.html).
* Common Weakness Enumeration: [CWE-247](https://cwe.mitre.org/data/definitions/247.html).
* Common Weakness Enumeration: [CWE-350](https://cwe.mitre.org/data/definitions/350.html).

55
docs/language/query-help/csharp/CookieWithOverlyBroadDomain.md
View File

@@ -1,55 +0,0 @@
# Cookie security: overly broad domain
```
ID: cs/web/broad-cookie-domain
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-287
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CookieWithOverlyBroadDomain.ql)
This rule finds cookies with an overly broad domain. Cookies with an overly broad domain, such as ".mybank.com", can be accessed by all web applications deployed on this domain and its sub-domains. A cookie with sensitive data, but with too broad a domain, could hence be read and tampered with by a less secure and untrusted application.
## Recommendation
Precisely define the domain of the web application for which this cookie is valid.
## Example
In this example `cookie1` is accessible from online-bank.com. `cookie2` is accessible from ebanking.online-bank.com and any subdomains of ebanking.online-bank.com.
```csharp
class CookieWithOverlyBroadDomain
{
static public void AddCookie()
{
HttpCookie cookie1 = new HttpCookie("sessionID");
cookie1.Domain = "online-bank.com";
HttpCookie cookie2 = new HttpCookie("sessionID");
cookie2.Domain = ".ebanking.online-bank.com";
}
}
```
In the following example `cookie` is only accessible from ebanking.online-bank.com which is much more secure.
```csharp
class CookieWithOverlyBroadDomainFix
{
static public void AddCookie()
{
HttpCookie cookie = new HttpCookie("sessionID");
cookie.Domain = "ebanking.online-bank.com";
}
}
```
## References
* MSDN: [HttpCookie.Domain Property](http://msdn.microsoft.com/en-us/library/system.web.httpcookie.domain.aspx).
* Common Weakness Enumeration: [CWE-287](https://cwe.mitre.org/data/definitions/287.html).

52
docs/language/query-help/csharp/CookieWithOverlyBroadPath.md
View File

@@ -1,52 +0,0 @@
# Cookie security: overly broad path
```
ID: cs/web/broad-cookie-path
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-287
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CookieWithOverlyBroadPath.ql)
This rule finds cookies with an overly broad path. Cookies with an overly broad path, such as the root context path ("/"), can be accessed by all web applications on the same domain name. A cookie with sensitive data, but with too broad a path, could hence be read and tampered by a less secure and untrusted application.
## Recommendation
Precisely define the path of the web application for which this cookie is valid.
## Example
In this example the cookie will be accessible to all applications regardless of their path. Most likely some of these applications are less secure than others and do not even need to access the same cookies.
```csharp
class CookieWithOverlyBroadPath
{
static public void AddCookie()
{
HttpCookie cookie = new HttpCookie("sessionID");
cookie.Path = "/";
}
}
```
In the following example the cookie is only accessible to the web application at the "/ebanking" path.
```csharp
class CookieWithOverlyBroadPathFix
{
static public void AddCookie()
{
HttpCookie cookie = new HttpCookie("sessionID");
cookie.Path = "/ebanking";
}
}
```
## References
* MSDN: [HttpCookie.Path Property](http://msdn.microsoft.com/en-us/library/system.web.httpcookie.path.aspx).
* Common Weakness Enumeration: [CWE-287](https://cwe.mitre.org/data/definitions/287.html).

44
docs/language/query-help/csharp/DeserializedDelegate.md
View File

@@ -1,44 +0,0 @@
# Deserialized delegate
```
ID: cs/deserialized-delegate
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-502
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-502/DeserializedDelegate.ql)
Deserializing a delegate object may result in remote code execution, when an attacker can control the serialized data.
## Recommendation
Avoid deserializing delegate objects, if possible, or make sure that the serialized data cannot be controlled by an attacker.
## Example
In this example, a file stream is deserialized to a `Func<int>` object, using a `BinaryFormatter`. The file stream is a parameter of a public method, so depending on the calls to `InvokeSerialized`, this may or may not pose a security problem.
```csharp
using System;
using System.IO;
using System.Runtime.Serialization.Formatters.Binary;
class Bad
{
public static int InvokeSerialized(FileStream fs)
{
var formatter = new BinaryFormatter();
// BAD
var f = (Func<int>)formatter.Deserialize(fs);
return f();
}
}
```
## References
* Microsoft: [BinaryFormatter Class](https://docs.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter).
* Common Weakness Enumeration: [CWE-502](https://cwe.mitre.org/data/definitions/502.html).

22
docs/language/query-help/csharp/EmptyPasswordInConfigurationFile.md
View File

@@ -1,22 +0,0 @@
# Empty password in configuration file
```
ID: cs/empty-password-in-configuration
Kind: problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-258 external/cwe/cwe-862
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Configuration/EmptyPasswordInConfigurationFile.ql)
The use of an empty string as a password in a configuration file is not secure.
## Recommendation
Choose a proper password and encrypt it if you need to store it in the configuration file.
## References
* Common Weakness Enumeration: [CWE-258](https://cwe.mitre.org/data/definitions/258.html).
* Common Weakness Enumeration: [CWE-862](https://cwe.mitre.org/data/definitions/862.html).

22
docs/language/query-help/csharp/Encryption using ECB.md
View File

@@ -1,22 +0,0 @@
# Encryption using ECB
```
ID: cs/ecb-encryption
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-327
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/Encryption%20using%20ECB.ql)
ECB should not be used as a mode for encryption. It has dangerous weaknesses. Data is encrypted the same way every time meaning the same plaintext input will always produce the same cyphertext. This makes encrypted messages vulnerable to replay attacks.
## Recommendation
Use a different CypherMode.
## References
* Wikipedia, Block cypher modes of operation, [Electronic codebook (ECB)](https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Electronic_codebook_.28ECB.29).
* Common Weakness Enumeration: [CWE-327](https://cwe.mitre.org/data/definitions/327.html).

65
docs/language/query-help/csharp/ExceptionInformationExposure.md
View File

@@ -1,65 +0,0 @@
# Information exposure through an exception
```
ID: cs/information-exposure-through-exception
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-209 external/cwe/cwe-497
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-209/ExceptionInformationExposure.ql)
Software developers often add stack traces to error messages, as a debugging aid. Whenever that error message occurs for an end user, the developer can use the stack trace to help identify how to fix the problem. In particular, stack traces can tell the developer more about the sequence of events that led to a failure, as opposed to merely the final state of the software when the error occurred.
Unfortunately, the same information can be useful to an attacker. The sequence of class names in a stack trace can reveal the structure of the application as well as any internal components it relies on. Furthermore, the error message at the top of a stack trace can include information such as server-side file names and SQL code that the application relies on, allowing an attacker to fine-tune a subsequent injection attack.
## Recommendation
Send the user a more generic error message that reveals less information. Either suppress the stack trace entirely, or log it only on the server.
## Example
In the following example, an exception is handled in two different ways. In the first version, labeled BAD, the exception is sent back to the remote user by calling `ToString()`, and writing it to the response. As such, the user is able to see a detailed stack trace, which may contain sensitive information. In the second version, the error message is logged only on the server. That way, the developers can still access and use the error log, but remote users will not see the information.
```csharp
using System;
using System.Web;
public class StackTraceHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
try
{
doSomeWork();
}
catch (Exception ex)
{
// BAD: printing a stack trace back to the response
ctx.Response.Write(ex.ToString());
return;
}
try
{
doSomeWork();
}
catch (Exception ex)
{
// GOOD: log the stack trace, and send back a non-revealing response
log("Exception occurred", ex);
ctx.Response.Write("Exception occurred");
return;
}
}
}
```
## References
* OWASP: [Information Leak](https://www.owasp.org/index.php/Information_Leak_(information_disclosure)).
* Common Weakness Enumeration: [CWE-209](https://cwe.mitre.org/data/definitions/209.html).
* Common Weakness Enumeration: [CWE-497](https://cwe.mitre.org/data/definitions/497.html).

66
docs/language/query-help/csharp/ExposureInTransmittedData.md
View File

@@ -1,66 +0,0 @@
# Information exposure through transmitted data
```
ID: cs/sensitive-data-transmission
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-201
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-201/ExposureInTransmittedData.ql)
Transmitting sensitive data to the user is a potential security risk. Always ensure that transmitted data is intended for the user. For example, passwords and the contents of database exceptions are generally not appropriate to send to the user, as they reveal information that could be abused or exploited.
## Recommendation
Avoid transmitting passwords or exceptions to the user. Instead, create a more user-friendly message that does not contain potentially sensitive information. Technical errors should be written to a log file.
## Example
The following example shows the user password being sent back to the user.
```csharp
public class Handler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
try
{
...
}
catch (AuthenticationFailure ex)
{
ctx.Response.Write("Invalid password: " + password);
}
}
}
```
The following example shows a database exception being sent to the user. Exceptions can often contain unnecessary technical or sensitive information that should not be seen by the user.
```csharp
public class Handler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
try
{
...
}
catch (DbException ex)
{
ctx.Response.Write("Database error: " + ex.Message);
}
}
}
```
## References
* OWASP: [Sensitive Data Exposure](https://www.owasp.org/index.php/Top_10_2013-A6-Sensitive_Data_Exposure).
* Common Weakness Enumeration: [CWE-201](https://cwe.mitre.org/data/definitions/201.html).

44
docs/language/query-help/csharp/ExposureOfPrivateInformation.md
View File

@@ -1,44 +0,0 @@
# Exposure of private information
```
ID: cs/exposure-of-sensitive-information
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-359
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-359/ExposureOfPrivateInformation.ql)
Private information that is stored in an external location may be more vulnerable because that location may not be protected by the same access controls as other parts of the system.
Examples include log files, cookies and plain text storage on disk.
## Recommendation
Ensure that private information is only stored in secure data locations.
## Example
The following example shows some private data - an address - being passed to a HTTP handler. This private information is then stored in a log file. This log file on disk may be accessible to users that do not normally have access to this private data.
```csharp
using System.Text;
using System.Web;
using System.Web.Security;
public class PrivateInformationHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string address = ctx.Request.QueryString["Address1"];
logger.Info("User has address: " + address);
}
}
```
## References
* Common Weakness Enumeration: [CWE-359](https://cwe.mitre.org/data/definitions/359.html).

78
docs/language/query-help/csharp/HardcodedConnectionString.md
View File

@@ -1,78 +0,0 @@
# Hard-coded connection string with credentials
```
ID: cs/hardcoded-connection-string-credentials
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-259 external/cwe/cwe-321 external/cwe/cwe-798
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-798/HardcodedConnectionString.ql)
Including unencrypted hard-coded inbound or outbound authentication credentials within source code or configuration files is dangerous because the credentials may be easily discovered.
Source or configuration files containing hard-coded credentials may be visible to an attacker. For example, the source code may be open source, or it may be leaked or accidentally revealed. For applications shipped as binaries, the credentials may be accessible within the compiled assemblies.
For inbound authentication, hard-coded credentials may allow unauthorized access to the system. This is particularly problematic if the credential is hard-coded in the source code, because it cannot be disabled easily. For outbound authentication, the hard-coded credentials may provide an attacker with privileged information or unauthorized access to some other system.
## Recommendation
Remove hard-coded credentials, such as user names, passwords and certificates, from source code, placing them in configuration files or other data stores if necessary. If possible, store configuration files including credential data separately from the source code, in a secure location with restricted access.
For outbound authentication details, consider encrypting the credentials or the enclosing data stores or configuration files, and using permissions to restrict access.
For inbound authentication details, consider hashing passwords using standard library functions where possible. For example, Microsoft provide the class `Microsoft.AspNet.Identity.PasswordHasher`.
## Example
The following examples shows different types of inbound and outbound authentication.
In the first case, we accept a password from a remote user, and compare it against a plaintext string literal. If an attacker acquires the source code, or the assemblies, they can observe the password, and can log in to the system. Furthermore, if such an intrusion was discovered, the application would need to be recompiled in order to change the password.
In the second case, the password is compared to a hashed and salted password stored in a configuration file, using the Microsoft provided `PasswordHasher.VerifyHashedPassword`. In this case, access to the source code or the assembly would not reveal the password to an attacker. Even access to the configuration file containing the password hash and salt would be of little value to an attacker, as it is usually extremely difficult to reverse engineer the password from the hash and salt.
In the final case, a password is changed to a new, hard-coded value. If an attacker has access to the source code, they will be able to observe the new password.
```csharp
using Microsoft.AspNet.Identity;
using System;
using System.Web;
using System.Web.Security;
public class HardCodedCredentialHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string password = ctx.Request.QueryString["password"];
// BAD: Inbound authentication made by comparison to string literal
if (password == "myPa55word")
{
ctx.Response.Redirect("login");
}
string hashedPassword = loadPasswordFromSecretConfig();
// GOOD: Inbound authentication made by comparing to a hash password from a config
if (PasswordHasher.VerifyHashedPassword(hashedPassword, password))
{
ctx.Response.Redirect(VALID_REDIRECT);
}
// BAD: Set the password to a hardcoded string literal
MembershipUser user = loadMembershipUser();
user.ChangePassword(password, "myNewPa55word");
}
}
```
## References
* OWASP: [XSS Use of hard-coded password](https://www.owasp.org/index.php/Use_of_hard-coded_password).
* Microsoft Docs: [Preventing Open Redirection Attacks (C#)](https://docs.microsoft.com/en-us/aspnet/mvc/overview/security/preventing-open-redirection-attacks).
* Common Weakness Enumeration: [CWE-259](https://cwe.mitre.org/data/definitions/259.html).
* Common Weakness Enumeration: [CWE-321](https://cwe.mitre.org/data/definitions/321.html).
* Common Weakness Enumeration: [CWE-798](https://cwe.mitre.org/data/definitions/798.html).

78
docs/language/query-help/csharp/HardcodedCredentials.md
View File

@@ -1,78 +0,0 @@
# Hard-coded credentials
```
ID: cs/hardcoded-credentials
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-259 external/cwe/cwe-321 external/cwe/cwe-798
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-798/HardcodedCredentials.ql)
Including unencrypted hard-coded inbound or outbound authentication credentials within source code or configuration files is dangerous because the credentials may be easily discovered.
Source or configuration files containing hard-coded credentials may be visible to an attacker. For example, the source code may be open source, or it may be leaked or accidentally revealed. For applications shipped as binaries, the credentials may be accessible within the compiled assemblies.
For inbound authentication, hard-coded credentials may allow unauthorized access to the system. This is particularly problematic if the credential is hard-coded in the source code, because it cannot be disabled easily. For outbound authentication, the hard-coded credentials may provide an attacker with privileged information or unauthorized access to some other system.
## Recommendation
Remove hard-coded credentials, such as user names, passwords and certificates, from source code, placing them in configuration files or other data stores if necessary. If possible, store configuration files including credential data separately from the source code, in a secure location with restricted access.
For outbound authentication details, consider encrypting the credentials or the enclosing data stores or configuration files, and using permissions to restrict access.
For inbound authentication details, consider hashing passwords using standard library functions where possible. For example, Microsoft provide the class `Microsoft.AspNet.Identity.PasswordHasher`.
## Example
The following examples shows different types of inbound and outbound authentication.
In the first case, we accept a password from a remote user, and compare it against a plaintext string literal. If an attacker acquires the source code, or the assemblies, they can observe the password, and can log in to the system. Furthermore, if such an intrusion was discovered, the application would need to be recompiled in order to change the password.
In the second case, the password is compared to a hashed and salted password stored in a configuration file, using the Microsoft provided `PasswordHasher.VerifyHashedPassword`. In this case, access to the source code or the assembly would not reveal the password to an attacker. Even access to the configuration file containing the password hash and salt would be of little value to an attacker, as it is usually extremely difficult to reverse engineer the password from the hash and salt.
In the final case, a password is changed to a new, hard-coded value. If an attacker has access to the source code, they will be able to observe the new password.
```csharp
using Microsoft.AspNet.Identity;
using System;
using System.Web;
using System.Web.Security;
public class HardCodedCredentialHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string password = ctx.Request.QueryString["password"];
// BAD: Inbound authentication made by comparison to string literal
if (password == "myPa55word")
{
ctx.Response.Redirect("login");
}
string hashedPassword = loadPasswordFromSecretConfig();
// GOOD: Inbound authentication made by comparing to a hash password from a config
if (PasswordHasher.VerifyHashedPassword(hashedPassword, password))
{
ctx.Response.Redirect(VALID_REDIRECT);
}
// BAD: Set the password to a hardcoded string literal
MembershipUser user = loadMembershipUser();
user.ChangePassword(password, "myNewPa55word");
}
}
```
## References
* OWASP: [XSS Use of hard-coded password](https://www.owasp.org/index.php/Use_of_hard-coded_password).
* Microsoft Docs: [Preventing Open Redirection Attacks (C#)](https://docs.microsoft.com/en-us/aspnet/mvc/overview/security/preventing-open-redirection-attacks).
* Common Weakness Enumeration: [CWE-259](https://cwe.mitre.org/data/definitions/259.html).
* Common Weakness Enumeration: [CWE-321](https://cwe.mitre.org/data/definitions/321.html).
* Common Weakness Enumeration: [CWE-798](https://cwe.mitre.org/data/definitions/798.html).

22
docs/language/query-help/csharp/HeaderCheckingDisabled.md
View File

@@ -1,22 +0,0 @@
# Header checking disabled
```
ID: cs/web/disabled-header-checking
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-113
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/HeaderCheckingDisabled.ql)
This rule finds places in the code where header checking is disabled. When header checking is enabled, which is the default, the `\r` or `\n` characters found in a response header are encoded to `%0d` and `%0a`. This defeats header-injection attacks by making the injected material part of the same header line. If you disable header checking, you open potential attack vectors against your client code.
## Recommendation
Do not disable header checking.
## References
* MSDN. [HttpRuntimeSection.EnableHeaderChecking Property](http://msdn.microsoft.com/en-us/library/system.web.configuration.httpruntimesection.enableheaderchecking.aspx).
* Common Weakness Enumeration: [CWE-113](https://cwe.mitre.org/data/definitions/113.html).

23
docs/language/query-help/csharp/InadequateRSAPadding.md
View File

@@ -1,23 +0,0 @@
# Weak encryption: inadequate RSA padding
```
ID: cs/inadequate-rsa-padding
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-327 external/cwe/cwe-780
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/InadequateRSAPadding.ql)
This query finds uses of RSA encryption without secure padding. Using PKCS#1 v1.5 padding can open up your application to several different attacks resulting in the exposure of the encryption key or the ability to determine plaintext from encrypted messages.
## Recommendation
Use the more secure PKCS#1 v2 (OAEP) padding.
## References
* Wikipedia. [RSA. Padding Schemes](http://en.wikipedia.org/wiki/RSA_(algorithm)#Padding_schemes).
* Common Weakness Enumeration: [CWE-327](https://cwe.mitre.org/data/definitions/327.html).
* Common Weakness Enumeration: [CWE-780](https://cwe.mitre.org/data/definitions/780.html).

66
docs/language/query-help/csharp/InsecureRandomness.md
View File

@@ -1,66 +0,0 @@
# Insecure randomness
```
ID: cs/insecure-randomness
Kind: path-problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-338
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/InsecureRandomness.ql)
Using a cryptographically weak pseudo-random number generator to generate a security-sensitive value, such as a password, makes it easier for an attacker to predict the value.
Pseudo-random number generators generate a sequence of numbers that only approximates the properties of random numbers. The sequence is not truly random because it is completely determined by a relatively small set of initial values, the seed. If the random number generator is cryptographically weak, then this sequence may be easily predictable through outside observations.
## Recommendation
Use a cryptographically secure pseudo-random number generator if the output is to be used in a security sensitive context. As a rule of thumb, a value should be considered "security sensitive" if predicting it would allow the attacker to perform an action that they would otherwise be unable to perform. For example, if an attacker could predict the random password generated for a new user, they would be able to log in as that new user.
For C#, `RNGCryptoServiceProvider` provides a cryptographically secure pseudo-random number generator. `Random` is not cryptographically secure, and should be avoided in security contexts. For contexts which are not security sensitive, `Random` may be preferable as it has a more convenient interface, and is likely to be faster.
For the specific use-case of generating passwords, consider `System.Web.Security.Membership.GeneratePassword`, which provides a cryptographically secure method of generating random passwords.
## Example
The following examples show different ways of generating a password.
In the first case, we generate a fresh password by appending a random integer to the end of a static string. The random number generator used (`Random`) is not cryptographically secure, so it may be possible for an attacker to predict the generated password.
In the second example, a cryptographically secure random number generator is used for the same purpose. In this case, it is much harder to predict the generated integers.
In the final example, the password is generated using the `Membership.GeneratePassword` library method, which uses a cryptographically secure random number generator to generate a random series of characters. This method should be preferred when generating passwords, if possible, as it avoids potential pitfalls when converting the output of a random number generator (usually an int or a byte) to a series of permitted characters.
```csharp
using System.Security.Cryptography;
using System.Web.Security;
string GeneratePassword()
{
// BAD: Password is generated using a cryptographically insecure RNG
Random gen = new Random();
string password = "mypassword" + gen.Next();
// GOOD: Password is generated using a cryptographically secure RNG
using (RNGCryptoServiceProvider crypto = new RNGCryptoServiceProvider())
{
byte[] randomBytes = new byte[sizeof(int)];
crypto.GetBytes(randomBytes);
password = "mypassword" + BitConverter.ToInt32(randomBytes);
}
// GOOD: Password is generated using a cryptographically secure RNG
password = Membership.GeneratePassword(12, 3);
return password;
}
```
## References
* Wikipedia. [Pseudo-random number generator](http://en.wikipedia.org/wiki/Pseudorandom_number_generator).
* MSDN. [RandomNumberGenerator](http://msdn.microsoft.com/en-us/library/system.security.cryptography.randomnumbergenerator.aspx).
* MSDN. [Membership.GeneratePassword](https://msdn.microsoft.com/en-us/library/system.web.security.membership.generatepassword(v=vs.110).aspx).
* Common Weakness Enumeration: [CWE-338](https://cwe.mitre.org/data/definitions/338.html).

50
docs/language/query-help/csharp/InsecureSQLConnection.md
View File

@@ -1,50 +0,0 @@
# Insecure SQL connection
```
ID: cs/insecure-sql-connection
Kind: path-problem
Severity: error
Precision: medium
Tags: security external/cwe/cwe-327
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-327/InsecureSQLConnection.ql)
SQL Server connections where the client is not enforcing the encryption in transit are susceptible to multiple attacks, including a man-in-the-middle, that would potentially compromise the user credentials and/or the TDS session.
## Recommendation
Ensure that the client code enforces the `Encrypt` option by setting it to `true` in the connection string.
## Example
The following example shows a SQL connection string that is not explicitly enabling the `Encrypt` setting to force encryption.
```csharp
using System.Data.SqlClient;
// BAD, Encrypt not specified
string connectString =
"Server=1.2.3.4;Database=Anything;Integrated Security=true;";
SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(connectString);
var conn = new SqlConnection(builder.ConnectionString);
```
The following example shows a SQL connection string that is explicitly enabling the `Encrypt` setting to force encryption in transit.
```csharp
using System.Data.SqlClient;
string connectString =
"Server=1.2.3.4;Database=Anything;Integrated Security=true;;Encrypt=true;";
SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(connectString);
var conn = new SqlConnection(builder.ConnectionString);
```
## References
* Microsoft, SQL Protocols blog: [Selectively using secure connection to SQL Server](https://blogs.msdn.microsoft.com/sql_protocols/2009/10/19/selectively-using-secure-connection-to-sql-server/).
* Microsoft: [SqlConnection.ConnectionString Property](https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlconnection.connectionstring(v=vs.110).aspx).
* Microsoft: [Using Connection String Keywords with SQL Server Native Client](https://msdn.microsoft.com/en-us/library/ms130822.aspx).
* Microsoft: [Setting the connection properties](https://msdn.microsoft.com/en-us/library/ms378988(v=sql.110).aspx).
* Common Weakness Enumeration: [CWE-327](https://cwe.mitre.org/data/definitions/327.html).

22
docs/language/query-help/csharp/InsufficientKeySize.md
View File

@@ -1,22 +0,0 @@
# Weak encryption: Insufficient key size
```
ID: cs/insufficient-key-size
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-327
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/InsufficientKeySize.ql)
This rule finds uses of encryption algorithms with too small a key size. Encryption algorithms are vulnerable to brute force attack when too small a key size is used.
## Recommendation
The key should be at least 1024-bit long when using RSA encryption, and 128-bit long when using symmetric encryption.
## References
* Wikipedia. [Key size](http://en.wikipedia.org/wiki/Key_size).
* Common Weakness Enumeration: [CWE-327](https://cwe.mitre.org/data/definitions/327.html).

85
docs/language/query-help/csharp/LDAPInjection.md
View File

@@ -1,85 +0,0 @@
# LDAP query built from user-controlled sources
```
ID: cs/ldap-injection
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-090
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-090/LDAPInjection.ql)
If an LDAP query is built using string concatenation, and the components of the concatenation include user input, a user is likely to be able to run malicious LDAP queries.
## Recommendation
If user input must be included in an LDAP query, it should be escaped to avoid a malicious user providing special characters that change the meaning of the query. If possible, use an existing library, such as the AntiXSS library.
## Example
In the following examples, the code accepts an "organization name" and a "username" from the user, which it uses to query LDAP to access a "type" property.
The first example concatenates the unvalidated and unencoded user input directly into both the DN (Distinguished Name) and the search filter used for the LDAP query. A malicious user could provide special characters to change the meaning of these queries, and search for a completely different set of values.
The second example uses the Microsoft AntiXSS library to encode the user values before they are included in the DN and search filters. This ensures the meaning of the query cannot be changed by a malicious user.
```csharp
using Microsoft.Security.Application.Encoder
using System;
using System.DirectoryServices;
using System.Web;
public class LDAPInjectionHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string userName = ctx.Request.QueryString["username"];
string organizationName = ctx.Request.QueryString["organization_name"];
// BAD: User input used in DN (Distinguished Name) without encoding
string ldapQuery = "LDAP://myserver/OU=People,O=" + organizationName;
using (DirectoryEntry root = new DirectoryEntry(ldapQuery))
{
// BAD: User input used in search filter without encoding
DirectorySearcher ds = new DirectorySearcher(root, "username=" + userName);
SearchResult result = ds.FindOne();
if (result != null)
{
using (DirectoryEntry user = result.getDirectoryEntry())
{
ctx.Response.Write(user.Properties["type"].Value)
}
}
}
// GOOD: Organization name is encoded before being used in DN
string safeOrganizationName = Encoder.LdapDistinguishedNameEncode(organizationName);
string safeLDAPQuery = "LDAP://myserver/OU=People,O=" + safeOrganizationName;
using (DirectoryEntry root = new DirectoryEntry(safeLDAPQuery))
{
// GOOD: User input is encoded before being used in search filter
string safeUserName = Encoder.LdapFilterEncode(userName);
DirectorySearcher ds = new DirectorySearcher(root, "username=" + safeUserName);
SearchResult result = ds.FindOne();
if (result != null)
{
using (DirectoryEntry user = result.getDirectoryEntry())
{
ctx.Response.Write(user.Properties["type"].Value)
}
}
}
}
}
```
## References
* OWASP: [LDAP Injection Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/LDAP_Injection_Prevention_Cheat_Sheet.html).
* OWASP: [Preventing LDAP Injection in Java](https://www.owasp.org/index.php/Preventing_LDAP_Injection_in_Java).
* AntiXSS doc: [LdapFilterEncode](http://www.nudoq.org/#!/Packages/AntiXSS/AntiXssLibrary/Encoder/M/LdapFilterEncode).
* AntiXSS doc: [LdapDistinguishedNameEncode](http://www.nudoq.org/#!/Packages/AntiXSS/AntiXssLibrary/Encoder/M/LdapDistinguishedNameEncode).
* Common Weakness Enumeration: [CWE-90](https://cwe.mitre.org/data/definitions/90.html).

66
docs/language/query-help/csharp/LocalUnvalidatedArithmetic.md
View File

@@ -1,66 +0,0 @@
# Unvalidated local pointer arithmetic
```
ID: cs/unvalidated-local-pointer-arithmetic
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-119 external/cwe/cwe-120 external/cwe/cwe-122 external/cwe/cwe-788
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-119/LocalUnvalidatedArithmetic.ql)
It is dangerous to use the result of a virtual method call in pointer arithmetic without validation if external users can provide their own implementation of the virtual method. For example, if the analyzed project is distributed as a library or framework, then the end-user could provide a new implementation that returns any value.
## Recommendation
Always validate the result of virtual methods calls before performing pointer arithmetic to avoid reading or writing outside the bounds of an allocated buffer.
## Example
In this example, we write to a given element of an array, using an instance of the `PossiblyOverridableClass` to determine which element to write to.
In the first case, the `GetElementNumber` method is called, and the result is used in pointer arithmetic without any validation. If the user can define a subtype of `PossiblyOverridableClass`, they can create an implementation of `GetElementNumber` that returns an invalid element number. This would lead to a write occurring outside the bounds of the `charArray`.
In the second case, the result of `GetElementNumber` is stored, and confirmed to be within the bounds of the array. Note that it is not sufficient to check that it is smaller than the length. We must also ensure that it's greater than zero, to prevent writes to locations before the buffer as well as afterwards.
```csharp
public class PossiblyOverridable
{
public virtual int GetElementNumber()
{
// By default returns 0, which is safe
return 0;
}
}
public class PointerArithmetic
{
public unsafe void WriteToOffset(PossiblyOverridable possiblyOverridable,
char[] charArray)
{
fixed (char* charPointer = charArray)
{
// BAD: Unvalidated use of virtual method call result in pointer arithmetic
char* newCharPointer = charPointer + possiblyOverridable.GetElementNumber();
*newCharPointer = 'A';
// GOOD: Check that the number is viable
int number = possiblyOverridable.GetElementNumber();
if (number >= 0 && number < charArray.Length)
{
char* newCharPointer2 = charPointer + number;
*newCharPointer = 'A';
}
}
}
}
```
## References
* Microsoft: [Unsafe Code and Pointers](https://msdn.microsoft.com/en-us/library/t2yzs44b.aspx).
* Common Weakness Enumeration: [CWE-119](https://cwe.mitre.org/data/definitions/119.html).
* Common Weakness Enumeration: [CWE-120](https://cwe.mitre.org/data/definitions/120.html).
* Common Weakness Enumeration: [CWE-122](https://cwe.mitre.org/data/definitions/122.html).
* Common Weakness Enumeration: [CWE-788](https://cwe.mitre.org/data/definitions/788.html).

54
docs/language/query-help/csharp/LogForging.md
View File

@@ -1,54 +0,0 @@
# Log entries created from user input
```
ID: cs/log-forging
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-117
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-117/LogForging.ql)
If unsanitized user input is written to a log entry, a malicious user may be able to forge new log entries.
Forgery can occur if a user provides some input with characters that are interpreted when the log output is displayed. If the log is displayed as a plain text file, then new line characters can be used by a malicious user. If the log is displayed as HTML, then arbitrary HTML may be include to spoof log entries.
## Recommendation
User input should be suitably encoded before it is logged.
If the log entries are plain text then line breaks should be removed from user input, using `String.Replace` or similar. Care should also be taken that user input is clearly marked in log entries, and that a malicious user cannot cause confusion in other ways.
For log entries that will be displayed in HTML, user input should be HTML encoded using `HttpServerUtility.HtmlEncode` or similar before being logged, to prevent forgery and other forms of HTML injection.
## Example
In the following example, a user name, provided by the user, is logged using a logging framework. In the first case, it is logged without any sanitization. In the second case, `String.Replace` is used to ensure no line endings are present in the user input.
```csharp
using Microsoft.Extensions.Logging;
using System;
using System.IO;
using System.Web;
public class LogForgingHandler : IHttpHandler
{
private ILogger logger;
public void ProcessRequest(HttpContext ctx)
{
String username = ctx.Request.QueryString["username"];
// BAD: User input logged as-is
logger.Warn(username + " log in requested.");
// GOOD: User input logged with new-lines removed
logger.Warn(username.Replace(Environment.NewLine, "") + " log in requested");
}
}
```
## References
* OWASP: [Log Injection](https://www.owasp.org/index.php/Log_Injection).
* Common Weakness Enumeration: [CWE-117](https://cwe.mitre.org/data/definitions/117.html).

71
docs/language/query-help/csharp/MissingASPNETGlobalErrorHandler.md
View File

@@ -1,71 +0,0 @@
# Missing global error handler
```
ID: cs/web/missing-global-error-handler
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-12 external/cwe/cwe-248
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-248/MissingASPNETGlobalErrorHandler.ql)
`Web.config` files that set the `customErrors` mode to `Off` and do not provide an `Application_Error` method in the `global.asax.cs` file rely on the default error pages, which leak information such as stack traces.
## Recommendation
Set the `customErrors` to `On` to prevent the default error page from being displayed, or to `RemoteOnly` to only show the default error page when the application is accessed locally. Alternatively, provide an implementation of the `Application_Error` method in the `global.asax.cs` page.
## Example
The following example shows a `Web.config` file in which the custom errors mode has been set to `Off`.
```none
<?xml version="1.0" encoding="utf-8" ?>
<configuration>
<system.web>
<customErrors mode="Off">
...
</customErrors>
</system.web>
</configuration>
```
This can be fixed either by specifying a different mode, such as `On`, in the `Web.config` file:
```none
<?xml version="1.0" encoding="utf-8" ?>
<configuration>
<system.web>
<customErrors mode="On">
...
</customErrors>
</system.web>
</configuration>
```
or by defining an `Application_Error` method in the `global.asax.cs` file:
```csharp
using System;
using System.Web;
namespace WebApp
{
public class Global : HttpApplication
{
void Application_Error(object sender, EventArgs e)
{
// Handle errors here
}
}
}
```
## References
* Common Weakness Enumeration: [CWE-12](https://cwe.mitre.org/data/definitions/12.html).
* Common Weakness Enumeration: [CWE-248](https://cwe.mitre.org/data/definitions/248.html).

54
docs/language/query-help/csharp/MissingAntiForgeryTokenValidation.md
View File

@@ -1,54 +0,0 @@
# Missing cross-site request forgery token validation
```
ID: cs/web/missing-token-validation
Kind: problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-352
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-352/MissingAntiForgeryTokenValidation.ql)
Web applications that use tokens to prevent cross-site request forgery (CSRF) should validate the tokens for all Http POST requests.
Although login and authentication methods are not vulnerable to traditional CSRF attacks, they still need to be protected with a token or other mitigation. This because an unprotected login page can be used by an attacker to force a login using an account controlled by the attacker. Subsequent requests to the site are then made using this account, without the user being aware that this is the case. This can result in the user associating private information with the attacker-controlled account.
## Recommendation
The appropriate attribute should be added to this method to ensure the anti-forgery token is validated when this action method is called. If using the MVC-provided anti-forgery framework this will be the `[ValidateAntiForgeryToken]` attribute.
Alternatively, you may consider including a global filter that applies token validation to all POST requests.
## Example
In the following example an ASP.NET MVC `Controller` is using the `[ValidateAntiForgeryToken]` attribute to mitigate against CSRF attacks. It has been applied correctly to the `UpdateDetails` method. However, this attribute has not been applied to the `Login` method. This should be fixed by adding this attribute.
```csharp
using System.Web.Mvc;
public class HomeController : Controller
{
// BAD: Anti forgery token has been forgotten
[HttpPost]
public ActionResult Login()
{
return View();
}
// GOOD: Anti forgery token is validated
[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult UpdateDetails()
{
return View();
}
}
```
## References
* Wikipedia: [Cross-Site Request Forgery](https://en.wikipedia.org/wiki/Cross-site_request_forgery).
* Microsoft Docs: [XSRF/CSRF Prevention in ASP.NET MVC and Web Pages](https://docs.microsoft.com/en-us/aspnet/mvc/overview/security/xsrfcsrf-prevention-in-aspnet-mvc-and-web-pages).
* Common Weakness Enumeration: [CWE-352](https://cwe.mitre.org/data/definitions/352.html).

56
docs/language/query-help/csharp/MissingXFrameOptions.md
View File

@@ -1,56 +0,0 @@
# Missing X-Frame-Options HTTP header
```
ID: cs/web/missing-x-frame-options
Kind: problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-451 external/cwe/cwe-829
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-451/MissingXFrameOptions.ql)
Web sites that do not specify the `X-Frame-Options` HTTP header may be vulnerable to UI redress attacks ("clickjacking"). In these attacks, the vulnerable site is loaded in a frame on an attacker-controlled site which uses opaque or transparent layers to trick the user into unintentionally clicking a button or link on the vulnerable site.
## Recommendation
Set the `X-Frame-Options` HTTP header to `DENY`, to instruct web browsers to block attempts to load the site in a frame. Alternatively, if framing is needed in certain circumstances, specify `SAMEORIGIN` or `ALLOW FROM: ...` to limit the ability to frame the site to pages from the same origin, or from an allowed whitelist of trusted domains.
For ASP.NET web applications, the header may be specified either in the `Web.config` file, using the `<customHeaders>` tag, or within the source code of the application using the `HttpResponse.AddHeader` method. In general, prefer specifying the header in the `Web.config` file to ensure it is added to all requests. If adding it to the source code, ensure that it is added unconditionally to all requests. For example, add the header in the `Application_BeginRequest` method in the `global.asax` file.
## Example
The following example shows how to specify the `X-Frame-Options` header within the `Web.config` file for ASP.NET:
```none
<?xml version="1.0" encoding="utf-8" ?>
<configuration>
<system.web>
</system.web>
<system.webServer>
<httpProtocol>
<customHeaders>
<add name="X-Frame-Options" value="SAMEORIGIN" />
</customHeaders>
</httpProtocol>
</system.webServer>
</configuration>
```
This next example shows how to specify the `X-Frame-Options` header within the `global.asax` file for ASP.NET application:
```csharp
protected void Application_BeginRequest(object sender, EventArgs e)
{
HttpContext.Current.Response.AddHeader("X-Frame-Options", "DENY");
}
```
## References
* OWASP: [Clickjacking Defense Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Clickjacking_Defense_Cheat_Sheet.html).
* Mozilla: [X-Frame-Options](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options)
* Common Weakness Enumeration: [CWE-451](https://cwe.mitre.org/data/definitions/451.html).
* Common Weakness Enumeration: [CWE-829](https://cwe.mitre.org/data/definitions/829.html).

72
docs/language/query-help/csharp/MissingXMLValidation.md
View File

@@ -1,72 +0,0 @@
# Missing XML validation
```
ID: cs/xml/missing-validation
Kind: path-problem
Severity: recommendation
Precision: high
Tags: security external/cwe/cwe-112
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-112/MissingXMLValidation.ql)
If unsanitized user input is processed as XML, it should be validated against a known schema. If no validation occurs, or if the validation relies on the schema or DTD specified in the document itself, then the XML document may contain any data in any form, which may invalidate assumptions the program later makes.
## Recommendation
All XML provided by a user should be validated against a known schema when it is processed.
If using `XmlReader.Create`, you should always pass an instance of `XmlReaderSettings`, with the following properties:
* `ValidationType` must be set to `Schema`. If this property is unset, no validation occurs. If it is set to `DTD`, the document is only validated against the DTD specified in the user-provided document itself - which could be specified as anything by a malicious user.
* `ValidationFlags` must not include `ProcessInlineSchema` or `ProcessSchemaLocation`. These flags allow a user to provide their own inline schema or schema location for validation, allowing a malicious user to bypass the known schema validation.
## Example
In the following example, text provided by a user is loaded using `XmlReader.Create`. In the first three examples, insufficient validation occurs, because either no validation is specified, or validation is only specified against a DTD provided by the user, or the validation permits a user to provide an inline schema. In the final example, a known schema is provided, and validation is set, using an instance of `XmlReaderSettings`. This ensures that the user input is properly validated against the known schema.
```csharp
using System;
using System.IO;
using System.Web;
using System.Xml;
using System.Xml.Schema;
public class MissingXmlValidationHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
String userProvidedXml = ctx.Request.QueryString["userProvidedXml"];
// BAD: User provided XML is processed without any validation,
// because there is no settings instance configured.
XmlReader.Create(new StringReader(userProvidedXml));
// BAD: User provided XML is processed without any validation,
// because the settings instance specifies DTD as the ValidationType
XmlReaderSettings badSettings = new XmlReaderSettings();
badSettings.ValidationType = ValidationType.DTD;
XmlReader.Create(new StringReader(userProvidedXml), badSettings);
// BAD: User provided XML is processed with validation, but the ProcessInlineSchema
// option is specified, so an attacker can provide their own schema to validate
// against.
XmlReaderSettings badInlineSettings = new XmlReaderSettings();
badInlineSettings.ValidationType = ValidationType.Schema;
badInlineSettings.ValidationFlags |= XmlSchemaValidationFlags.ProcessInlineSchema;
XmlReader.Create(new StringReader(userProvidedXml), badInlineSettings);
// GOOD: User provided XML is processed with validation
XmlReaderSettings goodSettings = new XmlReaderSettings();
goodSettings.ValidationType = ValidationType.Schema;
goodSettings.Schemas = new XmlSchemaSet() { { "urn:my-schema", "my.xsd" } };
XmlReader.Create(new StringReader(userProvidedXml), goodSettings);
}
}
```
## References
* Microsoft: [XML Schema (XSD) Validation with XmlSchemaSet](https://msdn.microsoft.com/en-us/library/3740e0b5(v=vs.110).aspx).
* Common Weakness Enumeration: [CWE-112](https://cwe.mitre.org/data/definitions/112.html).

23
docs/language/query-help/csharp/PasswordInConfigurationFile.md
View File

@@ -1,23 +0,0 @@
# Password in configuration file
```
ID: cs/password-in-configuration
Kind: problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-13 external/cwe/cwe-256 external/cwe/cwe-313
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Configuration/PasswordInConfigurationFile.ql)
Storing a plaintext password in a configuration file allows anyone who can read the file to access the password-protected resources. Therefore it is a common attack vector.
## Recommendation
Passwords stored in configuration files should be encrypted.
## References
* Common Weakness Enumeration: [CWE-13](https://cwe.mitre.org/data/definitions/13.html).
* Common Weakness Enumeration: [CWE-256](https://cwe.mitre.org/data/definitions/256.html).
* Common Weakness Enumeration: [CWE-313](https://cwe.mitre.org/data/definitions/313.html).

21
docs/language/query-help/csharp/PersistentCookie.md
View File

@@ -1,21 +0,0 @@
# Cookie security: persistent cookie
```
ID: cs/web/persistent-cookie
Kind: problem
Severity: warning
Precision: high
Tags: security external/cwe/cwe-539
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/PersistentCookie.ql)
This rule finds cookies that are made to expire in more than 5 minutes from now. Cookies are usually non-persistent, in which case they reside in the browser's memory only. However, by setting an expiration date in the future, cookies can be made persistent and are then written to disk to survive the browser restarts. If a persistent cookie is set to expire in a fairly distant future, it is easier for an attacker to steal its data.
## Recommendation
Do not put sensitive information in persistent cookies.
## References
* Common Weakness Enumeration: [CWE-539](https://cwe.mitre.org/data/definitions/539.html).

57
docs/language/query-help/csharp/ReDoS.md
View File

@@ -1,57 +0,0 @@
# Denial of Service from comparison of user input against expensive regex
```
ID: cs/redos
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-730 external/cwe/cwe-400
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-730/ReDoS.ql)
Matching user input against a regular expression which takes exponential time in the worst case can allow a malicious user to perform a Denial of Service ("DoS") attack by crafting input that takes a long time to execute.
Most regular expression engines, including the C# standard library implementation, are designed to work with an extended regular expression syntax. Although this provides flexibility for the user, it can prevent the engine from constructing an efficient implementation of the matcher in all circumstances. In particular, the "worst case time complexity" (see the references) of certain regular expressions may be "exponential". This would allow a malicious user to provide some input which causes the regular expression to take a very long time to execute.
Typically, a regular expression is vulnerable to this attack if it applies repetition to a sub-expression which itself is repeated, or contains overlapping options. For example, `(a+)+` is vulnerable to a string such as `aaaaaaaaaaaaaaaaaaaaaaaaaaab`. More information about the precise circumstances can be found in the references.
## Recommendation
Modify the regular expression to avoid the exponential worst case time. If this is not possible, then a timeout should be used to avoid a denial of service. For C# applications, a timeout can be provided to the `Regex` constructor. Alternatively, apply a global timeout by setting the `REGEX_DEFAULT_MATCH_TIMEOUT` application domain property, using the `AppDomain.SetData` method.
## Example
The following example shows a HTTP request parameter that is matched against a regular expression which has exponential worst case performance. In the first case, it is matched without a timeout, which can lead to a denial of service. In the second case, a timeout is used to cancel the evaluation of the regular expression after 1 second.
```csharp
using System;
using System.Web;
using System.Text.RegularExpressions;
public class ReDoSHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string userInput = ctx.Request.QueryString["userInput"];
// BAD: User input is matched against a regex with exponential worst case behavior
new Regex("^([a-z]*)*$").Match(userInput);
// GOOD: Regex is given a timeout to avoid DoS
new Regex("^([a-z]*)*$",
RegexOptions.IgnoreCase,
TimeSpan.FromSeconds(1)).Match(userInput);
}
}
```
## References
* OWASP: [Regular expression Denial of Service - ReDoS](https://www.owasp.org/index.php/Regular_expression_Denial_of_Service_-_ReDoS).
* Wikipedia: [ReDoS](https://en.wikipedia.org/wiki/ReDoS).
* Wikipedia: [Time complexity](https://en.wikipedia.org/wiki/Time_complexity).
* Common Weakness Enumeration: [CWE-730](https://cwe.mitre.org/data/definitions/730.html).
* Common Weakness Enumeration: [CWE-400](https://cwe.mitre.org/data/definitions/400.html).

56
docs/language/query-help/csharp/RegexInjection.md
View File

@@ -1,56 +0,0 @@
# Regular expression injection
```
ID: cs/regex-injection
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-730 external/cwe/cwe-400
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-730/RegexInjection.ql)
Constructing a regular expression with unsanitized user input is dangerous as a malicious user may be able to modify the meaning of the expression. In particular, such a user may be able to provide a regular expression fragment that takes exponential time in the worst case, and use that to perform a Denial of Service attack.
## Recommendation
For user input that is intended to be referenced as a string literal in a regular expression, use the `Regex.Escape` method to escape any special characters. If the regular expression is intended to be configurable by the user, then a timeout should be used to avoid Denial of Service attacks. For C# applications, a timeout can be provided to the `Regex` constructor. Alternatively, apply a global timeout by setting the `REGEX_DEFAULT_MATCH_TIMEOUT` application domain property, using the `AppDomain.SetData` method.
## Example
The following example shows a HTTP request parameter that is used as a regular expression, and matched against another request parameter.
In the first case, the regular expression is used without a timeout, and the user-provided regex is not escaped. If a malicious user provides a regex that has exponential worst case performance, then this could lead to a Denial of Service.
In the second case, the user input is escaped using `Regex.Escape` before being included in the regular expression. This ensures that the user cannot insert characters which have a special meaning in regular expressions.
```csharp
using System;
using System.Web;
using System.Text.RegularExpressions;
public class RegexInjectionHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string name = ctx.Request.QueryString["name"];
string userInput = ctx.Request.QueryString["userInput"];
// BAD: Unsanitized user input is used to construct a regular expression
new Regex("^" + name + "=.*$").Match(userInput);
// GOOD: User input is sanitized before constructing the regex
string safeName = Regex.Escape(name);
new Regex("^" + safeName + "=.*$").Match(userInput);
}
}
```
## References
* OWASP: [Regular expression Denial of Service - ReDoS](https://www.owasp.org/index.php/Regular_expression_Denial_of_Service_-_ReDoS).
* Wikipedia: [ReDoS](https://en.wikipedia.org/wiki/ReDoS).
* Common Weakness Enumeration: [CWE-730](https://cwe.mitre.org/data/definitions/730.html).
* Common Weakness Enumeration: [CWE-400](https://cwe.mitre.org/data/definitions/400.html).

46
docs/language/query-help/csharp/RequireSSL.md
View File

@@ -1,46 +0,0 @@
# 'requireSSL' attribute is not set to true
```
ID: cs/web/requiressl-not-set
Kind: problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-319 external/cwe/cwe-614
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-614/RequireSSL.ql)
Sensitive data that is transmitted using HTTP is vulnerable to being read by a third party. By default, web forms and cookies are sent via HTTP, not HTTPS. This setting can be changed by setting the `requireSSL` attribute to `"true"` in `Web.config`.
## Recommendation
When using web forms, ensure that `Web.config` contains a `<forms>` element with the attribute `requireSSL="true"`.
When using cookies, ensure that SSL is used, either via the `<forms>` attribute above, or the `<httpCookies>` element, with the attribute `requireSSL="true"`. It is also possible to require cookies to use SSL programmatically, by setting the property `System.Web.HttpCookie.Secure` to `true`.
## Example
The following example shows where to specify `requireSSL="true"` in a `Web.config` file.
```none
<?xml version="1.0" encoding="utf-8" ?>
<configuration>
<system.web>
<authentication>
<forms
requireSSL="true"
... />
</authentication>
<httpCookies
requireSSL="true"
... />
</system.web>
</configuration>
```
## References
* MSDN: [HttpCookie.Secure Property](https://msdn.microsoft.com/en-us/library/system.web.httpcookie.secure(v=vs.110).aspx), [FormsAuthentication.RequireSSL Property](https://msdn.microsoft.com/en-us/library/system.web.security.formsauthentication.requiressl(v=vs.110).aspx), [forms Element for authentication](https://msdn.microsoft.com/en-us/library/1d3t3c61(v=vs.100).aspx), [httpCookies Element](https://msdn.microsoft.com/library/ms228262%28v=vs.100%29.aspx).
* Common Weakness Enumeration: [CWE-319](https://cwe.mitre.org/data/definitions/319.html).
* Common Weakness Enumeration: [CWE-614](https://cwe.mitre.org/data/definitions/614.html).

59
docs/language/query-help/csharp/ResourceInjection.md
View File

@@ -1,59 +0,0 @@
# Resource injection
```
ID: cs/resource-injection
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-099
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-099/ResourceInjection.ql)
If a resource descriptor is built using string concatenation, and the components of the concatenation include user input, a user may be able to hijack the resource which is loaded.
## Recommendation
If user input must be included in a resource descriptor, it should be escaped to avoid a malicious user providing special characters that change the meaning of the descriptor. If possible, use an existing library to either escape or construct the resource.
For data connections within sub namespaces of `System.Data`, a connection builder class is provided. For example, a connection string which is to be passed to `System.Data.SqlClient.SqlConnection` can be constructed safely using an instance of `System.Data.SqlClient.SqlConnectionStringBuilder`.
## Example
In the following examples, the code accepts a user name from the user, which it uses to create a connection string for an SQL database.
The first example concatenates the unvalidated and unencoded user input directly into the connection string. A malicious user could provide special characters to change the meaning of the connection string, and connect to a completely different server.
The second example uses the `SqlConnectionStringBuilder` to construct the connection string and therefore prevents a malicious user modifying the meaning of the connection string.
```csharp
using System.Data.SqlClient;
using System.Web;
public class ResourceInjectionHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string userName = ctx.Request.QueryString["userName"];
// BAD: Direct use of user input in a connection string passed to SqlConnection
string connectionString = "server=(local);user id=" + userName + ";password= pass;";
SqlConnection sqlConnectionBad = new SqlConnection(connectionString);
// GOOD: Use SqlConnectionStringBuilder to safely include user input in a connection string
SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder();
builder["Data Source"] = "(local)";
builder["integrated Security"] = true;
builder["user id"] = userName;
SqlConnection sqlConnectionGood = new SqlConnection(builder.ConnectionString);
}
}
```
## References
* OWASP: [Resource Injection](https://www.owasp.org/index.php/Resource_Injection).
* MSDN: [Building Connection Strings](https://msdn.microsoft.com/en-us/library/ms254947(v=vs.80).aspx).
* MSDN: [Securing Connection Strings](https://msdn.microsoft.com/en-us/library/89211k9b(VS.80).aspx).
* Common Weakness Enumeration: [CWE-99](https://cwe.mitre.org/data/definitions/99.html).

83
docs/language/query-help/csharp/RuntimeChecksBypass.md
View File

@@ -1,83 +0,0 @@
# Serialization check bypass
```
ID: cs/serialization-check-bypass
Kind: problem
Severity: warning
Precision: medium
Tags: security external/cwe/cwe-20
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-020/RuntimeChecksBypass.ql)
Fields that are deserialized should be validated, otherwise the deserialized object could contain invalid data.
This query finds cases where a field is validated in a constructor, but not in a deserialization method. This is an indication that the deserialization method is missing a validation step.
## Recommendation
If a field needs to be validated, then ensure that validation is also performed during deserialization.
## Example
The following example has the validation of the `Age` field in the constructor but not in the deserialization method:
```csharp
using System;
using System.Runtime.Serialization;
[Serializable]
public class PersonBad : ISerializable
{
public int Age;
public PersonBad(int age)
{
if (age < 0)
throw new ArgumentException(nameof(age));
Age = age;
}
[OnDeserializing]
void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context)
{
Age = info.GetInt32("age"); // BAD - write is unsafe
}
}
```
The problem is fixed by adding validation to the deserialization method as follows:
```csharp
using System;
using System.Runtime.Serialization;
[Serializable]
public class PersonGood : ISerializable
{
public int Age;
public PersonGood(int age)
{
if (age < 0)
throw new ArgumentException(nameof(age));
Age = age;
}
[OnDeserializing]
void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context)
{
int age = info.GetInt32("age");
if (age < 0)
throw new SerializationException(nameof(Age));
Age = age; // GOOD - write is safe
}
}
```
## References
* OWASP: [Data Validation](https://www.owasp.org/index.php/Data_Validation).
* Common Weakness Enumeration: [CWE-20](https://cwe.mitre.org/data/definitions/20.html).

86
docs/language/query-help/csharp/SecondOrderSqlInjection.md
View File

@@ -1,86 +0,0 @@
# SQL query built from stored user-controlled sources
```
ID: cs/second-order-sql-injection
Kind: path-problem
Severity: error
Precision: medium
Tags: security external/cwe/cwe-089
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-089/SecondOrderSqlInjection.ql)
If a SQL query is built using string concatenation, and the components of the concatenation include user input, a user is likely to be able to run malicious database queries.
## Recommendation
Usually, it is better to use a prepared statement than to build a complete query with string concatenation. A prepared statement can include a parameter, written as either a question mark (`?`) or with an explicit name (`@parameter`), for each part of the SQL query that is expected to be filled in by a different value each time it is run. When the query is later executed, a value must be supplied for each parameter in the query.
It is good practice to use prepared statements for supplying parameters to a query, whether or not any of the parameters are directly traceable to user input. Doing so avoids any need to worry about quoting and escaping.
## Example
In the following example, the code runs a simple SQL query in three different ways.
The first way involves building a query, `query1`, by concatenating a user-supplied text box value with some string literals. The text box value can include special characters, so this code allows for SQL injection attacks.
The second way uses a stored procedure, `ItemsStoredProcedure`, with a single parameter (`@category`). The parameter is then given a value by calling `Parameters.Add`. This version is immune to injection attacks, because any special characters are not given any special treatment.
The third way builds a query, `query2`, with a single string literal that includes a parameter (`@category`). The parameter is then given a value by calling `Parameters.Add`. This version is immune to injection attacks, because any special characters are not given any special treatment.
```csharp
using System.Data;
using System.Data.SqlClient;
using System.Web.UI.WebControls;
class SqlInjection
{
TextBox categoryTextBox;
string connectionString;
public DataSet GetDataSetByCategory()
{
// BAD: the category might have SQL special characters in it
using (var connection = new SqlConnection(connectionString))
{
var query1 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
+ categoryTextBox.Text + "' ORDER BY PRICE";
var adapter = new SqlDataAdapter(query1, connection);
var result = new DataSet();
adapter.Fill(result);
return result;
}
// GOOD: use parameters with stored procedures
using (var connection = new SqlConnection(connectionString))
{
var adapter = new SqlDataAdapter("ItemsStoredProcedure", connection);
adapter.SelectCommand.CommandType = CommandType.StoredProcedure;
var parameter = new SqlParameter("category", categoryTextBox.Text);
adapter.SelectCommand.Parameters.Add(parameter);
var result = new DataSet();
adapter.Fill(result);
return result;
}
// GOOD: use parameters with dynamic SQL
using (var connection = new SqlConnection(connectionString))
{
var query2 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY="
+ "@category ORDER BY PRICE";
var adapter = new SqlDataAdapter(query2, connection);
var parameter = new SqlParameter("category", categoryTextBox.Text);
adapter.SelectCommand.Parameters.Add(parameter);
var result = new DataSet();
adapter.Fill(result);
return result;
}
}
}
```
## References
* MSDN: [How To: Protect From SQL Injection in ASP.NET](https://msdn.microsoft.com/en-us/library/ff648339.aspx).
* Common Weakness Enumeration: [CWE-89](https://cwe.mitre.org/data/definitions/89.html).

86
docs/language/query-help/csharp/SqlInjection.md
View File

@@ -1,86 +0,0 @@
# SQL query built from user-controlled sources
```
ID: cs/sql-injection
Kind: path-problem
Severity: error
Precision: high
Tags: security external/cwe/cwe-089
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-089/SqlInjection.ql)
If a SQL query is built using string concatenation, and the components of the concatenation include user input, a user is likely to be able to run malicious database queries.
## Recommendation
Usually, it is better to use a prepared statement than to build a complete query with string concatenation. A prepared statement can include a parameter, written as either a question mark (`?`) or with an explicit name (`@parameter`), for each part of the SQL query that is expected to be filled in by a different value each time it is run. When the query is later executed, a value must be supplied for each parameter in the query.
It is good practice to use prepared statements for supplying parameters to a query, whether or not any of the parameters are directly traceable to user input. Doing so avoids any need to worry about quoting and escaping.
## Example
In the following example, the code runs a simple SQL query in three different ways.
The first way involves building a query, `query1`, by concatenating a user-supplied text box value with some string literals. The text box value can include special characters, so this code allows for SQL injection attacks.
The second way uses a stored procedure, `ItemsStoredProcedure`, with a single parameter (`@category`). The parameter is then given a value by calling `Parameters.Add`. This version is immune to injection attacks, because any special characters are not given any special treatment.
The third way builds a query, `query2`, with a single string literal that includes a parameter (`@category`). The parameter is then given a value by calling `Parameters.Add`. This version is immune to injection attacks, because any special characters are not given any special treatment.
```csharp
using System.Data;
using System.Data.SqlClient;
using System.Web.UI.WebControls;
class SqlInjection
{
TextBox categoryTextBox;
string connectionString;
public DataSet GetDataSetByCategory()
{
// BAD: the category might have SQL special characters in it
using (var connection = new SqlConnection(connectionString))
{
var query1 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
+ categoryTextBox.Text + "' ORDER BY PRICE";
var adapter = new SqlDataAdapter(query1, connection);
var result = new DataSet();
adapter.Fill(result);
return result;
}
// GOOD: use parameters with stored procedures
using (var connection = new SqlConnection(connectionString))
{
var adapter = new SqlDataAdapter("ItemsStoredProcedure", connection);
adapter.SelectCommand.CommandType = CommandType.StoredProcedure;
var parameter = new SqlParameter("category", categoryTextBox.Text);
adapter.SelectCommand.Parameters.Add(parameter);
var result = new DataSet();
adapter.Fill(result);
return result;
}
// GOOD: use parameters with dynamic SQL
using (var connection = new SqlConnection(connectionString))
{
var query2 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY="
+ "@category ORDER BY PRICE";
var adapter = new SqlDataAdapter(query2, connection);
var parameter = new SqlParameter("category", categoryTextBox.Text);
adapter.SelectCommand.Parameters.Add(parameter);
var result = new DataSet();
adapter.Fill(result);
return result;
}
}
}
```
## References
* MSDN: [How To: Protect From SQL Injection in ASP.NET](https://msdn.microsoft.com/en-us/library/ff648339.aspx).
* Common Weakness Enumeration: [CWE-89](https://cwe.mitre.org/data/definitions/89.html).

45
docs/language/query-help/csharp/StoredCommandInjection.md
View File

@@ -1,45 +0,0 @@
# Uncontrolled command line from stored user input
```
ID: cs/stored-command-line-injection
Kind: path-problem
Severity: error
Precision: medium
Tags: correctness security external/cwe/cwe-078 external/cwe/cwe-088
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-078/StoredCommandInjection.ql)
Code that passes user input directly to `System.Diagnostic.Process.Start`, or some other library routine that executes a command, allows the user to execute malicious code.
## Recommendation
If possible, use hard-coded string literals to specify the command to run or library to load. Instead of passing the user input directly to the process or library function, examine the user input and then choose among hard-coded string literals.
If the applicable libraries or commands cannot be determined at compile time, then add code to verify that the user input string is safe before using it.
## Example
The following example shows code that takes a shell script that can be changed maliciously by a user, and passes it straight to `System.Diagnostic.Process.Start` without examining it first.
```csharp
using System;
using System.Web;
using System.Diagnostics;
public class CommandInjectionHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string param = ctx.Request.QueryString["param"];
Process.Start("process.exe", "/c " + param);
}
}
```
## References
* OWASP: [Command Injection](https://www.owasp.org/index.php/Command_Injection).
* Common Weakness Enumeration: [CWE-78](https://cwe.mitre.org/data/definitions/78.html).
* Common Weakness Enumeration: [CWE-88](https://cwe.mitre.org/data/definitions/88.html).

85
docs/language/query-help/csharp/StoredLDAPInjection.md
View File

@@ -1,85 +0,0 @@
# LDAP query built from stored user-controlled sources
```
ID: cs/stored-ldap-injection
Kind: path-problem
Severity: error
Precision: medium
Tags: security external/cwe/cwe-090
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-090/StoredLDAPInjection.ql)
If an LDAP query is built using string concatenation, and the components of the concatenation include user input, a user is likely to be able to run malicious LDAP queries.
## Recommendation
If user input must be included in an LDAP query, it should be escaped to avoid a malicious user providing special characters that change the meaning of the query. If possible, use an existing library, such as the AntiXSS library.
## Example
In the following examples, the code accepts an "organization name" and a "username" from the user, which it uses to query LDAP to access a "type" property.
The first example concatenates the unvalidated and unencoded user input directly into both the DN (Distinguished Name) and the search filter used for the LDAP query. A malicious user could provide special characters to change the meaning of these queries, and search for a completely different set of values.
The second example uses the Microsoft AntiXSS library to encode the user values before they are included in the DN and search filters. This ensures the meaning of the query cannot be changed by a malicious user.
```csharp
using Microsoft.Security.Application.Encoder
using System;
using System.DirectoryServices;
using System.Web;
public class LDAPInjectionHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string userName = ctx.Request.QueryString["username"];
string organizationName = ctx.Request.QueryString["organization_name"];
// BAD: User input used in DN (Distinguished Name) without encoding
string ldapQuery = "LDAP://myserver/OU=People,O=" + organizationName;
using (DirectoryEntry root = new DirectoryEntry(ldapQuery))
{
// BAD: User input used in search filter without encoding
DirectorySearcher ds = new DirectorySearcher(root, "username=" + userName);
SearchResult result = ds.FindOne();
if (result != null)
{
using (DirectoryEntry user = result.getDirectoryEntry())
{
ctx.Response.Write(user.Properties["type"].Value)
}
}
}
// GOOD: Organization name is encoded before being used in DN
string safeOrganizationName = Encoder.LdapDistinguishedNameEncode(organizationName);
string safeLDAPQuery = "LDAP://myserver/OU=People,O=" + safeOrganizationName;
using (DirectoryEntry root = new DirectoryEntry(safeLDAPQuery))
{
// GOOD: User input is encoded before being used in search filter
string safeUserName = Encoder.LdapFilterEncode(userName);
DirectorySearcher ds = new DirectorySearcher(root, "username=" + safeUserName);
SearchResult result = ds.FindOne();
if (result != null)
{
using (DirectoryEntry user = result.getDirectoryEntry())
{
ctx.Response.Write(user.Properties["type"].Value)
}
}
}
}
}
```
## References
* OWASP: [LDAP Injection Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/LDAP_Injection_Prevention_Cheat_Sheet.html).
* OWASP: [Preventing LDAP Injection in Java](https://www.owasp.org/index.php/Preventing_LDAP_Injection_in_Java).
* AntiXSS doc: [LdapFilterEncode](http://www.nudoq.org/#!/Packages/AntiXSS/AntiXssLibrary/Encoder/M/LdapFilterEncode).
* AntiXSS doc: [LdapDistinguishedNameEncode](http://www.nudoq.org/#!/Packages/AntiXSS/AntiXssLibrary/Encoder/M/LdapDistinguishedNameEncode).
* Common Weakness Enumeration: [CWE-90](https://cwe.mitre.org/data/definitions/90.html).

64
docs/language/query-help/csharp/StoredXPathInjection.md
View File

@@ -1,64 +0,0 @@
# Stored XPath injection
```
ID: cs/xml/stored-xpath-injection
Kind: path-problem
Severity: error
Precision: medium
Tags: security external/cwe/cwe-643
```
[Click to see the query in the CodeQL repository](https://github.com/github/codeql/tree/main/csharp/ql/src/Security%20Features/CWE-643/StoredXPathInjection.ql)
If an XPath expression is built using string concatenation, and the components of the concatenation include user input, a user is likely to be able to create a malicious XPath expression.
## Recommendation
If user input must be included in an XPath expression, pre-compile the query and use variable references to include the user input.
When using the `System.Xml.XPath` API, this can be done by creating a custom subtype of `System.Xml.Xsl.XsltContext`, and implementing `ResolveVariable(String,?String)` to return the user provided data. This custom context can be specified for a given `XPathExpression` using `XPathExpression.SetContext()`. For more details, see the "User Defined Functions and Variables" webpage in the list of references.
## Example
In the first example, the code accepts a user name specified by the user, and uses this unvalidated and unsanitized value in an XPath expression. This is vulnerable to the user providing special characters or string sequences that change the meaning of the XPath expression to search for different values.
In the second example, the XPath expression is a hard-coded string that specifies some variables, which are safely replaced at runtime using a custom `XsltContext` that looks up the variables in an `XsltArgumentList`.
```csharp
using System;
using System.Web;
using System.Xml.XPath;
public class XPathInjectionHandler : IHttpHandler
{
public void ProcessRequest(HttpContext ctx)
{
string userName = ctx.Request.QueryString["userName"];
// BAD: Use user-provided data directly in an XPath expression
string badXPathExpr = "//users/user[login/text()='" + userName + "']/home_dir/text()";
XPathExpression.Compile(badXPathExpr);
// GOOD: XPath expression uses variables to refer to parameters
string xpathExpression = "//users/user[login/text()=$username]/home_dir/text()";
XPathExpression xpath = XPathExpression.Compile(xpathExpression);
// Arguments are provided as a XsltArgumentList()
XsltArgumentList varList = new XsltArgumentList();
varList.AddParam("userName", string.Empty, userName);
// CustomContext is an application specific class, that looks up variables in the
// expression from the varList.
CustomContext context = new CustomContext(new NameTable(), varList)
xpath.SetContext(context);
}
}
```
## References
* OWASP: [Testing for XPath Injection](https://www.owasp.org/index.php?title=Testing_for_XPath_Injection_(OTG-INPVAL-010)).
* OWASP: [XPath Injection](https://www.owasp.org/index.php/XPATH_Injection).
* MSDN: [User Defined Functions and Variables](https://msdn.microsoft.com/en-us/library/dd567715.aspx).
* Common Weakness Enumeration: [CWE-643](https://cwe.mitre.org/data/definitions/643.html).

Some files were not shown because too many files have changed in this diff Show More