Merge pull request #17028 from geoffw0/cryptodoc

C++: Improve query doc advice for using encryption

Author: geoffw0

cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted.qhelp

@@ -10,11 +10,12 @@ contains sensitive data that could somehow be retrieved by an attacker.</p>
 </overview>
 <recommendation>
 
-<p>Use alternative platform-supplied functions that will not get optimized away. Examples of such
-functions include <code>memset_s</code>, <code>SecureZeroMemory</code>, and <code>bzero_explicit</code>.
-Alternatively, passing the <code>-fno-builtin-memset</code> option to the GCC/Clang compiler usually
-also prevents the optimization. Finally, you can use the public-domain <code>secure_memzero</code> function
-(see references below). This function, however, is not guaranteed to work on all platforms and compilers.</p>
+<p>Use <code>memset_s</code> (from C11) instead of <code>memset</code>, as <code>memset_s</code> will not
+get optimized away. Alternatively use platform-supplied functions such as <code>SecureZeroMemory</code> or
+<code>bzero_explicit</code> that make the same guarantee. Passing the <code>-fno-builtin-memset</code>
+option to the GCC/Clang compiler usually also prevents the optimization. Finally, you can use the
+public-domain <code>secure_memzero</code> function (see references below). This function, however, is not
+guaranteed to work on all platforms and compilers.</p>
 
 </recommendation>
 <example>

cpp/ql/src/Security/CWE/CWE-311/CleartextStorage.c

@@ -1,12 +1,31 @@
-void writeCredentials() {
-  char *password = "cleartext password";
-  FILE* file = fopen("credentials.txt", "w");
-  
+#include <sodium.h>
+#include <stdio.h>
+#include <string.h>
+
+void writeCredentialsBad(FILE *file, const char *cleartextCredentials) {
   // BAD: write password to disk in cleartext
-  fputs(password, file);
-  
-  // GOOD: encrypt password first
-  char *encrypted = encrypt(password);
-  fputs(encrypted, file);
+  fputs(cleartextCredentials, file);
 }
 
+int writeCredentialsGood(FILE *file, const char *cleartextCredentials, const unsigned char *key, const unsigned char *nonce) {
+  size_t credentialsLen = strlen(cleartextCredentials);
+  size_t ciphertext_len = crypto_secretbox_MACBYTES + credentialsLen;
+  unsigned char *ciphertext = malloc(ciphertext_len);
+  if (!ciphertext) {
+    logError();
+    return -1;
+  }
+
+  // encrypt the password first
+  if (crypto_secretbox_easy(ciphertext, (const unsigned char *)cleartextCredentials, credentialsLen, nonce, key) != 0) {
+    free(ciphertext);
+    logError();
+    return -1;
+  }
+
+  // GOOD: write encrypted password to disk
+  fwrite(ciphertext, 1, ciphertext_len, file);
+
+  free(ciphertext);
+  return 0;
+}

cpp/ql/src/Security/CWE/CWE-311/CleartextStorage.inc.qhelp

@@ -19,15 +19,20 @@ cleartext.</p>
 <example>
 
 <p>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 
+the credentials are simply stored in cleartext. In the 'GOOD' case, the credentials are encrypted before
 storing them.</p>
 
 <sample src="CleartextStorage.c" />
 
+<p>Note that for the 'GOOD' example to work we need to link against an encryption library (in this case libsodium),
+initialize it with a call to <code>sodium_init</code>, and create the key and nonce with
+<code>crypto_secretbox_keygen</code> and <code>randombytes_buf</code> respectively. We also need to store those
+details securely so they can be used for decryption.</p>
+
 </example>
 <references>
 
-<li>M. Dowd, J. McDonald and J. Schuhm, <i>The Art of Software Security Assessment</i>, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.</li> 
+<li>M. Dowd, J. McDonald and J. Schuhm, <i>The Art of Software Security Assessment</i>, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.</li>
 <li>M. Howard and D. LeBlanc, <i>Writing Secure Code</i>, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.</li>
 
 

cpp/ql/src/Security/CWE/CWE-313/CleartextSqliteDatabase.c

@@ -1,6 +1,6 @@
 
 void bad(void) {
-  char *password = "cleartext password";
+  const char *password = "cleartext password";
   sqlite3 *credentialsDB;
   sqlite3_stmt *stmt;
 
@@ -16,14 +16,15 @@ void bad(void) {
   }
 }
 
-void good(void) {
-  char *password = "cleartext password";
+void good(const char *secretKey) {
+  const 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);
+    std::string setKeyString = std::string("PRAGMA key = '") + secretKey + "'";
+    sqlite3_exec(credentialsDB, setKeyString.c_str(), 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);
@@ -33,4 +34,3 @@ void good(void) {
     }
   }
 }
-

cpp/ql/src/Security/CWE/CWE-313/CleartextSqliteDatabase.qhelp

@@ -20,10 +20,12 @@ In the 'GOOD' case, the database (and thus the credentials) are encrypted.</p>
 
 <sample src="CleartextSqliteDatabase.c" />
 
+<p>Note that for the 'GOOD' example to work we need to provide a secret key. Secure key generation and storage is required.</p>
+
 </example>
 <references>
 
-<li>M. Dowd, J. McDonald and J. Schuhm, <i>The Art of Software Security Assessment</i>, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.</li> 
+<li>M. Dowd, J. McDonald and J. Schuhm, <i>The Art of Software Security Assessment</i>, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.</li>
 <li>M. Howard and D. LeBlanc, <i>Writing Secure Code</i>, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.</li>