From 5a9061e45b0ad9cc22e11e002e664a08adb34e21 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Sat, 21 May 2022 14:19:34 +0000 Subject: [PATCH 1/9] create new branchihsinme-patch-102 in fork --- .../BufferAccessWithIncorrectLengthValue.cpp | 8 ++ ...BufferAccessWithIncorrectLengthValue.qhelp | 23 ++++++ .../BufferAccessWithIncorrectLengthValue.ql | 73 +++++++++++++++++++ ...ferAccessWithIncorrectLengthValue.expected | 1 + ...BufferAccessWithIncorrectLengthValue.qlref | 1 + .../CWE/CWE-805/semmle/tests/test.cpp | 31 ++++++++ 6 files changed, 137 insertions(+) create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.cpp create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.qhelp create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/BufferAccessWithIncorrectLengthValue.expected create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/BufferAccessWithIncorrectLengthValue.qlref create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/test.cpp diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.cpp new file mode 100644 index 00000000000..650e2c4062b --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.cpp @@ -0,0 +1,8 @@ + +... +char buf[256]; +X509_NAME_oneline(X509_get_subject_name(peer),buf,sizeof(buf)); // GOOD +... +char buf[256]; +X509_NAME_oneline(X509_get_subject_name(peer),buf,1024); // BAD +... diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.qhelp new file mode 100644 index 00000000000..561679780ff --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.qhelp @@ -0,0 +1,23 @@ + + + +

Using a size argument that is larger than the buffer size will result in an out-of-memory and possibly overflow. You need to limit the value of the length argument.

+ + + + +

The following example shows the use of a function with and without an error in the size argument.

+ + + + + +
  • + CERT Coding Standard: + ARR38-C. Guarantee that library functions do not form invalid pointers - SEI CERT C Coding Standard - Confluence. +
    • + +     +     \ No newline at end of file diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql new file mode 100644 index 00000000000..1e424d77fed --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql @@ -0,0 +1,73 @@ +/** + * @name Buffer access with incorrect length value. + * @description Incorrect use of the length argument in some functions will result in out-of-memory accesses. + * @kind problem + * @id cpp/buffer-access-with-incorrect-length-value + * @problem.severity warning + * @precision medium + * @tags correctness + * security + * external/cwe/cwe-805 + */ + +import cpp +import semmle.code.cpp.valuenumbering.GlobalValueNumbering + +/** Holds for a function `f`, which has an argument at index `bpos` that points to a buffer and an argument at index `spos` that points to a size. */ +predicate numberArgument(Function f, int bpos, int spos) { + f.hasGlobalOrStdName([ + "X509_NAME_oneline", "SSL_CIPHER_description", "SSL_get_shared_ciphers", + "SSL_export_keying_material_early", "SSL_export_keying_material", "SSL_set_alpn_protos", + "SSL_CTX_set_alpn_protos", "SSL_read", "SSL_read_ex", "SSL_read_early_data", + "SSL_bytes_to_cipher_list", "SSL_write", "SSL_SESSION_set1_master_key", + "SSL_CTX_set_session_id_context", "BIO_gets", "BIO_read", "BIO_read_ex", "BIO_write", + "BIO_write_ex", "BIO_ctrl", "BN_bn2binpad", "BN_signed_bn2bin", "BN_signed_bn2lebin", + "EVP_PKEY_get_default_digest_name", "EVP_DigestUpdate", "EVP_PKEY_CTX_set1_tls1_prf_secret", + "EVP_KDF_derive", "EVP_CIPHER_CTX_get_updated_iv", "EVP_PKEY_get_group_name", "EVP_MAC_init", + "write", "read", "send", "sendto", "recv", "recvfrom", "strerror_r" + ]) and + bpos = 1 and + spos = 2 + or + f.hasGlobalOrStdName(["X509_NAME_get_text_by_NID", "EVP_PKEY_get_utf8_string_param"]) and + bpos = 2 and + spos = 3 + or + f.hasGlobalOrStdName([ + "BIO_snprintf", "BN_signed_lebin2bn", "BIO_new_mem_buf", "BN_lebin2bn", "BN_bin2bn", + "EVP_read_pw_string", "EVP_read_pw_string", "strftime", "strnlen", "fgets", "snprintf", + "vsnprintf" + ]) and + bpos = 0 and + spos = 1 + or + f.hasGlobalOrStdName(["AES_ige_encrypt", "memchr"]) and bpos = 0 and spos = 2 + or + f.hasGlobalOrStdName(["EVP_MAC_final"]) and bpos = 1 and spos = 3 + or + f.hasGlobalOrStdName(["OBJ_obj2txt"]) and bpos = 2 and spos = 1 + or + f.hasGlobalOrStdName(["EVP_CIPHER_CTX_ctrl"]) and bpos = 3 and spos = 2 + or + f.hasGlobalOrStdName(["EVP_PKEY_get_octet_string_param", "getnameinfo"]) and bpos = 2 and spos = 3 + or + f.hasGlobalOrStdName([ + "EVP_DecryptUpdate", "EVP_EncryptUpdate", "EVP_PKEY_encrypt", "EVP_PKEY_sign", + "EVP_CipherUpdate" + ]) and + bpos = 3 and + spos = 4 + or + f.hasGlobalOrStdName(["getnameinfo"]) and bpos = 4 and spos = 5 +} + +from FunctionCall fc +where + exists(ArrayType at, int b, int s, Variable vr | + numberArgument(fc.getTarget(), b, s) and + fc.getArgument(s).getValue().toInt() > at.getByteSize() and + vr.getADeclarationEntry().getType() = at and + fc.getArgument(b).(VariableAccess).getTarget() = vr + ) +select fc, + "Access beyond the bounds of the allocated memory is possible, the size argument used is greater than the size of the buffer." diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/BufferAccessWithIncorrectLengthValue.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/BufferAccessWithIncorrectLengthValue.expected new file mode 100644 index 00000000000..d0d9fbad694 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/BufferAccessWithIncorrectLengthValue.expected @@ -0,0 +1 @@ +| test.cpp:27:5:27:21 | call to X509_NAME_oneline | Access beyond the bounds of the allocated memory is possible, the size argument used is greater than the size of the buffer. | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/BufferAccessWithIncorrectLengthValue.qlref b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/BufferAccessWithIncorrectLengthValue.qlref new file mode 100644 index 00000000000..6cbb5527211 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/BufferAccessWithIncorrectLengthValue.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/test.cpp new file mode 100644 index 00000000000..26c33abab65 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-805/semmle/tests/test.cpp @@ -0,0 +1,31 @@ +struct X509_NAME {}; +struct SSL {}; +struct X509 {}; + +char * X509_NAME_oneline(X509_NAME *a,char *buf,int size); +X509 *SSL_get_peer_certificate(const SSL *ssl); +X509_NAME *X509_get_subject_name(const X509 *x); +char *strcasestr(char *a, char *b); + +bool goodTest1(SSL *ssl,char *text) +{ + X509 *peer; + char buf[256]; + if( peer = SSL_get_peer_certificate(ssl)) + { + X509_NAME_oneline(X509_get_subject_name(peer),buf,sizeof(buf)); // GOOD + if((char*)strcasestr(buf,text)) return true; + } + return false; +} +bool badTest1(SSL *ssl,char *text) +{ + X509 *peer; + char buf[256]; + if( peer = SSL_get_peer_certificate(ssl)) + { + X509_NAME_oneline(X509_get_subject_name(peer),buf,1024); // BAD + if((char*)strcasestr(buf,text)) return true; + } + return false; +} From df1ea5b67939295bcb8d91347e644111b83c73d3 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Sun, 29 May 2022 09:56:29 +0300 Subject: [PATCH 2/9] Update BufferAccessWithIncorrectLengthValue.qhelp --- .../CWE/CWE-805/BufferAccessWithIncorrectLengthValue.qhelp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.qhelp index 561679780ff..c67d2c08895 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.qhelp +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.qhelp @@ -3,7 +3,7 @@ "qhelp.dtd"> -

    Using a size argument that is larger than the buffer size will result in an out-of-memory and possibly overflow. You need to limit the value of the length argument.

    +

    Using a size argument that is larger than the buffer size will result in an out-of-bounds memory access and possibly overflow. You need to limit the value of the length argument.

     @@ -20,4 +20,4 @@ -     \ No newline at end of file + From 475e36e6fc32a2e2852bbd620cf8ec95453fb51a Mon Sep 17 00:00:00 2001 From: ihsinme Date: Sun, 29 May 2022 09:58:16 +0300 Subject: [PATCH 3/9] Update BufferAccessWithIncorrectLengthValue.ql --- .../Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql index 1e424d77fed..b7f60876fec 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql @@ -11,7 +11,6 @@ */ import cpp -import semmle.code.cpp.valuenumbering.GlobalValueNumbering /** Holds for a function `f`, which has an argument at index `bpos` that points to a buffer and an argument at index `spos` that points to a size. */ predicate numberArgument(Function f, int bpos, int spos) { From d489c12014a851e40167ab29bd5263f59cbda229 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Mon, 30 May 2022 12:26:26 +0300 Subject: [PATCH 4/9] Update BufferAccessWithIncorrectLengthValue.ql --- .../CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql index b7f60876fec..15b48467945 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql @@ -62,11 +62,10 @@ predicate numberArgument(Function f, int bpos, int spos) { from FunctionCall fc where - exists(ArrayType at, int b, int s, Variable vr | - numberArgument(fc.getTarget(), b, s) and - fc.getArgument(s).getValue().toInt() > at.getByteSize() and - vr.getADeclarationEntry().getType() = at and - fc.getArgument(b).(VariableAccess).getTarget() = vr + exists(ArrayType array, int bufArgPos, int sizeArgPos | + numberArgument(fc.getTarget(), bufArgPos, sizeArgPos) and + fc.getArgument(sizeArgPos).getValue().toInt() > array.getByteSize() and + fc.getArgument(bufArgPos).(VariableAccess).getTarget().getADeclarationEntry().getType() = array ) select fc, "Access beyond the bounds of the allocated memory is possible, the size argument used is greater than the size of the buffer." From 9499961a9c5e2a0012fbdec00bf87e3f15ecddac Mon Sep 17 00:00:00 2001 From: ihsinme Date: Mon, 30 May 2022 12:43:28 +0300 Subject: [PATCH 5/9] Update BufferAccessWithIncorrectLengthValue.ql --- .../CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql index 15b48467945..9acacc42b89 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql @@ -42,11 +42,11 @@ predicate numberArgument(Function f, int bpos, int spos) { or f.hasGlobalOrStdName(["AES_ige_encrypt", "memchr"]) and bpos = 0 and spos = 2 or - f.hasGlobalOrStdName(["EVP_MAC_final"]) and bpos = 1 and spos = 3 + f.hasGlobalOrStdName("EVP_MAC_final") and bpos = 1 and spos = 3 or - f.hasGlobalOrStdName(["OBJ_obj2txt"]) and bpos = 2 and spos = 1 + f.hasGlobalOrStdName("OBJ_obj2txt") and bpos = 2 and spos = 1 or - f.hasGlobalOrStdName(["EVP_CIPHER_CTX_ctrl"]) and bpos = 3 and spos = 2 + f.hasGlobalOrStdName("EVP_CIPHER_CTX_ctrl") and bpos = 3 and spos = 2 or f.hasGlobalOrStdName(["EVP_PKEY_get_octet_string_param", "getnameinfo"]) and bpos = 2 and spos = 3 or @@ -57,7 +57,7 @@ predicate numberArgument(Function f, int bpos, int spos) { bpos = 3 and spos = 4 or - f.hasGlobalOrStdName(["getnameinfo"]) and bpos = 4 and spos = 5 + f.hasGlobalOrStdName("getnameinfo") and bpos = 4 and spos = 5 } from FunctionCall fc From 5609d5200b625908101bc8d161058c1a223789ed Mon Sep 17 00:00:00 2001 From: ihsinme Date: Mon, 30 May 2022 20:41:39 +0300 Subject: [PATCH 6/9] Update cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com> --- .../CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql index 9acacc42b89..fa3a9a1a3fe 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql @@ -1,5 +1,5 @@ /** - * @name Buffer access with incorrect length value. + * @name Buffer access with incorrect length value * @description Incorrect use of the length argument in some functions will result in out-of-memory accesses. * @kind problem * @id cpp/buffer-access-with-incorrect-length-value From c1f0940b6afc701ece24985129fffd937eaf6b42 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Thu, 23 Jun 2022 12:50:59 +0300 Subject: [PATCH 7/9] Update cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com> --- .../CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql index fa3a9a1a3fe..9eea998c15a 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql @@ -64,8 +64,8 @@ from FunctionCall fc where exists(ArrayType array, int bufArgPos, int sizeArgPos | numberArgument(fc.getTarget(), bufArgPos, sizeArgPos) and - fc.getArgument(sizeArgPos).getValue().toInt() > array.getByteSize() and - fc.getArgument(bufArgPos).(VariableAccess).getTarget().getADeclarationEntry().getType() = array + fc.getArgument(pragma[only_bind_into](sizeArgPos)).getValue().toInt() > array.getByteSize() and + fc.getArgument(pragma[only_bind_into](bufArgPos)).(VariableAccess).getTarget().getADeclarationEntry().getType() = array ) select fc, "Access beyond the bounds of the allocated memory is possible, the size argument used is greater than the size of the buffer." From 65c9a7b2780695c27f31fe34dd6f46b23d09dea2 Mon Sep 17 00:00:00 2001 From: ihsinme Date: Sun, 26 Jun 2022 13:49:44 +0300 Subject: [PATCH 8/9] Update BufferAccessWithIncorrectLengthValue.ql --- .../CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql index 9eea998c15a..bd7763bf9e0 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql @@ -65,7 +65,11 @@ where exists(ArrayType array, int bufArgPos, int sizeArgPos | numberArgument(fc.getTarget(), bufArgPos, sizeArgPos) and fc.getArgument(pragma[only_bind_into](sizeArgPos)).getValue().toInt() > array.getByteSize() and - fc.getArgument(pragma[only_bind_into](bufArgPos)).(VariableAccess).getTarget().getADeclarationEntry().getType() = array + fc.getArgument(pragma[only_bind_into](bufArgPos)) + .(VariableAccess) + .getTarget() + .getADeclarationEntry() + .getType() = array ) select fc, "Access beyond the bounds of the allocated memory is possible, the size argument used is greater than the size of the buffer." From 1c35109675659bdd33f4f13ecfd081880e8a65cb Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Mon, 6 Feb 2023 20:31:26 +0100 Subject: [PATCH 9/9] C++: Add experimental tag to experimental query --- .../Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql index bd7763bf9e0..0683e81ddd3 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-805/BufferAccessWithIncorrectLengthValue.ql @@ -7,6 +7,7 @@ * @precision medium * @tags correctness * security + * experimental * external/cwe/cwe-805 */