Jump to: navigation, search

Difference between revisions of "StarlingX/Security/Banned C Functions"

(Guidance)
 
(18 intermediate revisions by the same user not shown)
Line 1: Line 1:
 
=== Guidance ===
 
=== Guidance ===
Prohibiting the use of banned functions is a good way to remove a significant number of potential code vulnerabilities from C and C++ code.  This list is the compiled library of known bad functions that should be removed to reduce vulnerabilities. It is derived from experience with real-world security bugs and focuses primarily on functions that can lead to buffer overruns (reference: [https://msdn.microsoft.com/en-us/library/bb288454.aspx msdn]).  Specifically, for starling X, the main guidelines are that:
+
Prohibiting the use of banned functions is a good way to remove a significant number of potential code vulnerabilities from C and C++ code.  This list is the compiled library of known bad functions that should be removed to reduce vulnerabilities. It is derived from experience with real-world security bugs and focuses primarily on functions that can lead to buffer overruns (reference: [https://msdn.microsoft.com/en-us/library/bb288454.aspx msdn]).   
Only functions in the standard C runtime library—libc—are mandated
+
 
Unbounded functions are banned unless specifically noted
+
Specifically, for Starling X, the main guidelines are that:
Stack allocation functions are banned unless specifically approved by the project Core
+
* Only functions in the standard C runtime library—libc—are mandated
 +
* Unbounded functions are banned unless specifically noted
 +
* Stack allocation functions are banned unless specifically approved by the project core
  
 
There is no requirement to retrofit existing upstream code to meet these guidelines.  A summary of the policy is provided below.
 
There is no requirement to retrofit existing upstream code to meet these guidelines.  A summary of the policy is provided below.
Line 13: Line 15:
 
|-
 
|-
 
|strcpy, wcscpy
 
|strcpy, wcscpy
|unbounded, banned; use strncpy
+
| style="color: white; background-color:#ff0000;" | unbounded, banned; use strncpy
 
|-
 
|-
 
|strncpy
 
|strncpy
Line 19: Line 21:
 
|-
 
|-
 
|strcat, wcscat
 
|strcat, wcscat
|unbounded, banned; use strncat
+
| style="color: white; background-color:#ff0000" | unbounded, banned; use strncat
 
|-
 
|-
 
|strncat
 
|strncat
|inspect for truncated output
+
|inspect for truncated output and buffer overflow
 
|-
 
|-
 
|sprintf, vsprintf
 
|sprintf, vsprintf
|unbounded, banned; use snprintf, vsnprintf
+
| style="color: white; background-color:#ff0000" | unbounded, banned; use snprintf, vsnprintf
 
|-
 
|-
 
|snprintf
 
|snprintf
Line 31: Line 33:
 
|-
 
|-
 
|vsnprintf
 
|vsnprintf
|banned except with approval from PSE. requires detailed inspection to avoid va_list pitfalls
+
| style="color: white; background-color:#ff8000;" | banned except with approval from core. requires detailed inspection to avoid va_list pitfalls
 +
 
 +
vsnprint() is typically used for custom logging functionality.  Given the flexibility of this function, it is easy to mismatch data types pushed on the stack for a va-list function and types pulled from the stack by the function.  The core needs to ensure that the format matches the variables passed to avoid mismatches.
 
|-
 
|-
 
|strtok
 
|strtok
|unbounded, banned; use strtok_r or strsep
+
| style="color: white; background-color:#ff0000" | unbounded, banned; use strtok_r or strsep
 
|-
 
|-
 
|strtok_r, strsep
 
|strtok_r, strsep
 
|Inspect for terminated input buffer
 
|Inspect for terminated input buffer
 
|-
 
|-
|sscanf, vsscanf
+
|scanf, sscanf, vscanf, vsscanf
|unbounded, banned
+
| style="color: white; background-color:#ff8000" | banned except with approval of core. Requires detailed inspection to ensure field widths are specified.  For unknown inputs, it is recommended to use strto* functions to avoid arithmetic overflows.
 
|-
 
|-
 
|gets
 
|gets
|unbounded, banned, use fgets() instead
+
| style="color: white; background-color:#ff0000" | unbounded, banned, use fgets() instead
 
|-
 
|-
 
|ato*
 
|ato*
|banned, use equivalent strto* functions
+
| style="color: white; background-color:#ff0000" | banned, use equivalent strto* functions
 
|-
 
|-
 
|*toa
 
|*toa
Line 52: Line 56:
 
|-
 
|-
 
|strlen, wcslen
 
|strlen, wcslen
|banned except static strings; use strnlen with max length constant
+
| style="color: white; background-color:#ff8000;" | banned except static strings; use strnlen with max length constant
 
|-
 
|-
 
|memcpy, memmove
 
|memcpy, memmove
Line 58: Line 62:
 
|-
 
|-
 
|alloca
 
|alloca
|banned except with approval of PSE. requires detailed inspection to avoid stack overflow
+
| style="color: white; background-color:#ff8000;" | banned except with approval of core. Requires detailed inspection to avoid stack overflow.  In general, it is preferable to allocate from the heap using malloc() or calloc(). 
 +
 
 +
alloca() returns a pointer to a buffer located on the current thread's stack vs. the heap like malloc(). It's often used in cases where performance is especially important, but data length is unknown beforehand, because allocation is a simple matter of moving the stack pointer for the currently executing function.  The dangers of using alloca() are stack space exhaustion or a buffer overflow which corrupts local variables. 
 +
 
 +
If alloca() is used, the designer needs to understand that:
 +
# alloca() makes no guarantees about alignment and in most cases provides no overflow canaries that may exist in heap buffers
 +
# Never use alloca() in a loop where the bounds are not known beforehand. There is no ability to free memory allocated with alloca() unless you return from the function. If a long function continually calls alloca() in a loop, there is a high risk of stack exhaustion.
 +
# The conversion of alloca() to malloc() or calloc() requires a free() calls to be added as well
 
|}
 
|}
  
Line 65: Line 76:
 
|+
 
|+
 
|Allowed w/Inspection
 
|Allowed w/Inspection
|Banned
+
| style="color: white; background-color:#ff0000" | Banned
|Banned w/Exceptions
+
| style="color: white; background-color:#ff8000;" | Banned w/Exceptions
 
|}
 
|}

Latest revision as of 16:04, 9 January 2019

Guidance

Prohibiting the use of banned functions is a good way to remove a significant number of potential code vulnerabilities from C and C++ code. This list is the compiled library of known bad functions that should be removed to reduce vulnerabilities. It is derived from experience with real-world security bugs and focuses primarily on functions that can lead to buffer overruns (reference: msdn).

Specifically, for Starling X, the main guidelines are that:

  • Only functions in the standard C runtime library—libc—are mandated
  • Unbounded functions are banned unless specifically noted
  • Stack allocation functions are banned unless specifically approved by the project core

There is no requirement to retrofit existing upstream code to meet these guidelines. A summary of the policy is provided below.

Func Status
strcpy, wcscpy unbounded, banned; use strncpy
strncpy inspect for unterminated/truncated output
strcat, wcscat unbounded, banned; use strncat
strncat inspect for truncated output and buffer overflow
sprintf, vsprintf unbounded, banned; use snprintf, vsnprintf
snprintf inspect for result fitting in buffer: snprintf(buf, size, ...) < size
vsnprintf banned except with approval from core. requires detailed inspection to avoid va_list pitfalls.

vsnprint() is typically used for custom logging functionality. Given the flexibility of this function, it is easy to mismatch data types pushed on the stack for a va-list function and types pulled from the stack by the function. The core needs to ensure that the format matches the variables passed to avoid mismatches.

strtok unbounded, banned; use strtok_r or strsep
strtok_r, strsep Inspect for terminated input buffer
scanf, sscanf, vscanf, vsscanf banned except with approval of core. Requires detailed inspection to ensure field widths are specified. For unknown inputs, it is recommended to use strto* functions to avoid arithmetic overflows.
gets unbounded, banned, use fgets() instead
ato* banned, use equivalent strto* functions
*toa Non-standard, inspect for output buffer length; prefer snprintf
strlen, wcslen banned except static strings; use strnlen with max length constant
memcpy, memmove allowed
alloca banned except with approval of core. Requires detailed inspection to avoid stack overflow. In general, it is preferable to allocate from the heap using malloc() or calloc().

alloca() returns a pointer to a buffer located on the current thread's stack vs. the heap like malloc(). It's often used in cases where performance is especially important, but data length is unknown beforehand, because allocation is a simple matter of moving the stack pointer for the currently executing function. The dangers of using alloca() are stack space exhaustion or a buffer overflow which corrupts local variables.

If alloca() is used, the designer needs to understand that:

  1. alloca() makes no guarantees about alignment and in most cases provides no overflow canaries that may exist in heap buffers
  2. Never use alloca() in a loop where the bounds are not known beforehand. There is no ability to free memory allocated with alloca() unless you return from the function. If a long function continually calls alloca() in a loop, there is a high risk of stack exhaustion.
  3. The conversion of alloca() to malloc() or calloc() requires a free() calls to be added as well

Color Coding

Allowed w/Inspection Banned Banned w/Exceptions