Bring over documentation for new rules.

Author: NateD-MSFT

src/microsoft/Likely Bugs/Format/NonConstantFormat-1-bad.c

@@ -0,0 +1,6 @@
+#include <stdio.h>
+int main(int argc, char** argv) {
+  for(int i = 1; i < argc; ++i) {
+    printf(argv[i]);
+  }
+}

src/microsoft/Likely Bugs/Format/NonConstantFormat-1-good.c

@@ -0,0 +1,6 @@
+#include <stdio.h>
+int main(int argc, char** argv) {
+  for(int i = 1; i < argc; ++i) {
+    printf("%s", argv[i]);
+  }
+}

src/microsoft/Likely Bugs/Format/NonConstantFormat-2-bad.c

@@ -0,0 +1,13 @@
+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;
+}

src/microsoft/Likely Bugs/Format/NonConstantFormat-2-good.c

@@ -0,0 +1,16 @@
+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;
+}

src/microsoft/Likely Bugs/Format/NonConstantFormat-2-ok.c

@@ -0,0 +1,12 @@
+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;
+}

src/microsoft/Likely Bugs/Format/NonConstantFormat.md

@@ -0,0 +1,98 @@
+# Non-constant format string
+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`. The analysis 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).

src/microsoft/Likely Bugs/Format/NonConstantFormat.qhelp

@@ -0,0 +1,59 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>The <code>printf</code> function, related functions like <code>sprintf</code> and <code>fprintf</code>,
+and other functions built atop <code>vprintf</code> 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 <code>%s</code> and <code>%02x</code>) 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.</p>
+
+</overview>
+<recommendation>
+<p>If the argument passed as a format string is meant to be a plain string rather than a format string,
+then pass <code>%s</code> as the format string, and pass the original argument as the sole trailing
+argument.</p>
+
+<p>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.</p>
+
+</recommendation>
+<example>
+<p>The following program is meant to echo its command line arguments:</p>
+<sample src="NonConstantFormat-1-bad.c" />
+<p>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 <code>%s</code> format string, as in the following program:</p>
+<sample src="NonConstantFormat-1-good.c" />
+
+</example>
+<example>
+<p>The following program defines a <code>log_with_timestamp</code> function:</p>
+<sample src="NonConstantFormat-2-bad.c" />
+<p>In the code that is visible, the reader can verify that <code>log_with_timestamp</code> 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:</p>
+<sample src="NonConstantFormat-2-ok.c" />
+<p>An alternative solution is to allow <code>log_with_timestamp</code> to accept format arguments:</p>
+<sample src="NonConstantFormat-2-good.c" />
+<p>In this formulation, the non-constant format string to <code>printf</code> has been replaced with
+a non-constant format string to <code>vprintf</code>. The analysis will no longer consider the body of
+<code>log_with_timestamp</code> to be a problem, and will instead check that every call to
+<code>log_with_timestamp</code> passes a constant format string.</p>
+
+</example>
+<references>
+
+
+<li>CERT C Coding
+Standard: <a href="https://www.securecoding.cert.org/confluence/display/c/FIO30-C.+Exclude+user+input+from+format+strings">FIO30-C. Exclude user input from format strings</a>.</li>
+<li>M. Howard, D. Leblanc, J. Viega, <i>19 Deadly Sins of Software Security: Programming Flaws and How to Fix Them</i>.</li>
+
+
+</references>
+</qhelp>

src/microsoft/Likely Bugs/Memory Management/ImproperNullTermination.md

@@ -0,0 +1,42 @@
+# Potential improper null termination
+Built-in C string functions such as `strcat` require that their input string arguments are null terminated. If the input string arguments are not null terminated, these functions will read/write beyond the length of the buffer containing the string, resulting in either buffer over-read or buffer overflow, respectively.
+
+
+## Recommendation
+Review the code and consider whether the variable that holds the string should have an initializer or whether some path through the program fails to null terminate the string.
+
+
+## Example
+The destination variable `dest` used in the call to `strcat` does not (necessarily) contain a null terminator. Consequently, the call to `strcat` may result in a buffer overflow.
+
+
+```cpp
+char source[100];
+memset(source, 'A', 100-1);
+source[100-1] = '\0'; // null terminate source
+
+char dest[200];
+memset(dest, 'B', 100-1);
+
+strcat(dest, source);
+```
+In the revised example, `dest` is properly null terminated before the the call to `strcat`.
+
+
+```cpp
+char source[100];
+memset(source, 'A', 100-1);
+source[100-1] = '\0'; // null terminate source
+
+char dest[200];
+memset(dest, 'B', 100-1);
+dest[100-1] = '\0'; // null terminate destination
+
+strcat(dest, source);
+```
+
+## References
+* B. Chess and J. West, *Secure Programming with Static Analysis*, 6.2 Maintaining the Null Terminator. Addison-Wesley. 2007.
+* Linux Programmer's Manual: [STRCAT(3)](http://man7.org/linux/man-pages/man3/strncat.3.html).
+* Common Weakness Enumeration: [CWE-170](https://cwe.mitre.org/data/definitions/170.html).
+* Common Weakness Enumeration: [CWE-665](https://cwe.mitre.org/data/definitions/665.html).

src/microsoft/Likely Bugs/Memory Management/ImproperNullTermination.qhelp

@@ -0,0 +1,47 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p> Built-in C string functions such as <code>strcat</code> require that their
+input string arguments are null terminated. If the input string arguments are
+not null terminated, these functions will read/write beyond the length of the
+buffer containing the string, resulting in either buffer over-read or buffer
+overflow, respectively.
+</p>
+
+</overview>
+<recommendation>
+
+<p>
+Review the code and consider whether the variable that holds the string should have
+an initializer or whether some path through the program fails to null terminate the
+string.
+</p>
+
+</recommendation>
+<example>
+<p>The destination variable <code>dest</code> used in the call to <code>strcat</code>
+does not (necessarily) contain a null terminator. Consequently, the call to <code>strcat</code>
+may result in a buffer overflow.
+</p>
+
+<sample src="ImproperNullTerminationBad.cpp" />
+
+<p>In the revised example, <code>dest</code> is properly null terminated before the
+the call to <code>strcat</code>.
+</p>
+
+<sample src="ImproperNullTerminationGood.cpp" />
+
+
+</example>
+<references>
+
+  <li>B. Chess and J. West, <em>Secure Programming with Static Analysis</em>, 6.2 Maintaining the Null Terminator. Addison-Wesley. 2007.</li>
+  <li>Linux Programmer's Manual: <a href="http://man7.org/linux/man-pages/man3/strncat.3.html">STRCAT(3)</a>.</li>
+
+</references>
+</qhelp>

src/microsoft/Likely Bugs/Memory Management/ImproperNullTerminationBad.cpp

@@ -0,0 +1,8 @@
+char source[100];
+memset(source, 'A', 100-1);
+source[100-1] = '\0'; // null terminate source
+
+char dest[200];
+memset(dest, 'B', 100-1);
+
+strcat(dest, source);