(xmlsec-core) Added max key length limit for PBKDF2 and updated comments (#1391)

Author: lsh123

scripts/build_coverity.sh

@@ -18,22 +18,17 @@ srcdir=$(cd "$script_pwd/.." && pwd)
 today=`date +%F-%H-%M-%S`
 tar_file="xmlsec1-$version-$today.tar.gz"
 
-# Restore the caller's working directory on any exit (including 'set -e'
-# failures), since the script builds in the source tree.
-trap 'cd "$cur_pwd"' EXIT
-
-echo "============== Building xmlsec"
-cd "$srcdir"
+echo "============== Building xmlsec: $PWD =============="
 # Regenerate the build system so that configure always matches the current
 # configure.ac, even if a stale configure was left over from a previous run.
-autoreconf -i -f
-./configure --enable-legacy-features --enable-ftp --enable-http --with-gcrypt
+autoreconf -i -f "$srcdir"
+"$srcdir/configure" --enable-legacy-features --enable-ftp --enable-http --with-gcrypt
 make clean
 rm -rf cov-int/
 cov-build --dir cov-int make -j4
 tar czvf "$tar_file" cov-int
 
-echo "============== Uploading to Coverity"
+echo "============== Uploading to Coverity =============="
 curl \
     --form token="$cov_token" \
     --form email="$cov_email" \

src/gcrypt/ciphers.c

@@ -307,13 +307,14 @@ xmlSecGCryptBlockCipherCtxFinal(xmlSecGCryptBlockCipherCtxPtr ctx,
     if(encrypt == 0) {
         xmlSecSize padding;
 
-        /* check padding.
+        /* Check padding:
          *
-         * Only the padding length carried in the last byte is validated (it must be
-         * non-zero and not exceed the block size); the remaining padding bytes are not
-         * checked for equality and the check is not constant-time. This simplified
-         * validation is a deliberate project-wide design choice, matching the OpenSSL
-         * backend. See XML Encryption specification for details. */
+         * XML Encryption (W3C) uses PKCS#5-style padding: a full block is appended
+         * whose last octet holds the padding length, while the remaining padding octets
+         * are filled with random bytes (see the encrypt branch above). This is NOT strict
+         * PKCS#7 (where every padding octet equals the length), so the other octets are
+         * deliberately not validated for equality See the XML Encryption specification
+         * for details. */
         padding = (xmlSecSize)outBuf[blockLen - 1];
         if((padding == 0) || (inSize < padding)) {
             xmlSecInvalidSizeLessThanError("Input data padding",

src/gnutls/signatures.c

@@ -382,6 +382,9 @@ xmlSecGnuTLSSignatureInitialize(xmlSecTransformPtr transform) {
         ctx->keyId       = xmlSecGnuTLSKeyDataGost2001Id;
         ctx->dgstAlgo    = GNUTLS_DIG_GOSTR_94;
         ctx->signAlgo    = GNUTLS_SIGN_GOST_94;
+        /* GOST R 34.10-94 (GOST_01) is a legacy algorithm that GnuTLS classifies as
+         * "insufficient security" under the default security profile. Without the
+         * GNUTLS_VERIFY_ALLOW_BROKEN flag, the signature verification would fail. */
         ctx->verifyFlags = GNUTLS_VERIFY_ALLOW_BROKEN;
         ctx->getPubKey   = xmlSecGnuTLSKeyDataGost2001GetPublicKey;
         ctx->getPrivKey  = xmlSecGnuTLSKeyDataGost2001GetPrivateKey;

src/transform_helpers.c

@@ -1431,6 +1431,10 @@ xmlSecTransformPbkdf2ParamsRead(xmlSecTransformPbkdf2ParamsPtr params, xmlNodePt
         xmlSecInternalError("xmlSecGetNodeContentAsSize(keyLength)", NULL);
         return(-1);
     }
+    if(params->keyLength > XMLSEC_PBKDF2_MAX_KEY_LENGTH) {
+        xmlSecInvalidSizeDataError("keyLength", params->keyLength, "too big", NULL);
+        return(-1);
+    }
 
     /* next is required PRF */
     cur = xmlSecGetNextElementNode(cur->next);

src/transform_helpers.h

@@ -204,6 +204,17 @@ XMLSEC_EXPORT int xmlSecTransformEdDSAReadContextString(xmlNodePtr node,
 #define XMLSEC_PBKDF2_MAX_ITERATIONS  ((xmlSecSize)1000000)
 #endif /* !defined(XMLSEC_PBKDF2_MAX_ITERATIONS) */
 
+/*
+ * The maximum derived key length (in bytes) accepted by
+ * xmlSecTransformPbkdf2ParamsRead(). Key lengths above this
+ * limit are rejected to protect against oversized derived-key
+ * buffer allocations (denial of service) when processing
+ * untrusted documents.
+ */
+#if !defined(XMLSEC_PBKDF2_MAX_KEY_LENGTH)
+#define XMLSEC_PBKDF2_MAX_KEY_LENGTH  ((xmlSecSize)4096)
+#endif /* !defined(XMLSEC_PBKDF2_MAX_KEY_LENGTH) */
+
 struct _xmlSecTransformPbkdf2Params {
     xmlSecBuffer salt;
     xmlSecSize iterationCount;