Fixed minor unit tests and build system issues

Makefile.am

@@ -104,6 +104,13 @@ if XMLSEC_APPS
 
 check: check-all check-fuzz check-info
 
+# Build the command line tool and the unit tests on demand so the check-* /
+# memcheck / perfcheck targets can be run directly in a fresh build dir,
+# before a full 'make' has built the subdirectories. The whole project is
+# built (not just apps/) so the library dependencies in src/ are available.
+$(TEST_APP) $(UNIT_TEST_APP):
+	$(MAKE) $(AM_MAKEFLAGS) all
+
 check-unit-tests: $(UNIT_TEST_APP)
 	@($(PRECHECK_COMMANDS) && $(ABS_BUILDDIR)/$(UNIT_TEST_APP))
 

apps/cmdline.c

@@ -10,6 +10,7 @@
  */
 #include <stdio.h>
 #include <stdlib.h>
+#include <ctype.h>
 #include <time.h>
 #include <string.h>
 #include <assert.h>
@@ -36,10 +37,8 @@ static int                      xmlSecAppCmdLineTimeParamRead   (const char* str
                                                                   int is_gmt_time);
 
 #if defined(_MSC_VER)
-#define XMLSEC_SCANF     sscanf_s
 #define XMLSEC_MKGMTIME  _mkgmtime
 #else /* defined(_MSC_VER) */
-#define XMLSEC_SCANF      sscanf
 #define XMLSEC_MKGMTIME  xmlSecAppGetGmtTime
 
 static time_t                   xmlSecAppGetGmtTime             (struct tm* timeptr);
@@ -441,51 +440,123 @@ xmlSecAppIsValidDate(int year, int month, int day) {
     return((day >= 1) && (day <= maxDay));
 }
 
+/**
+ * Reads 1 to maxDigits digits from *pp, advancing *pp past them.
+ * Returns the numeric value, or -1 if no digit is present.
+ */
+static int
+xmlSecAppCmdLineParseDigits(const char** pp, int maxDigits) {
+    int value = 0;
+    int count = 0;
+    const char* p = *pp;
+
+    while((count < maxDigits) && (*p >= '0') && (*p <= '9')) {
+        value = (value * 10) + (*p - '0');
+        p++;
+        count++;
+    }
+    if(count == 0) {
+        return(-1);
+    }
+    *pp = p;
+    return(value);
+}
+
 static int
 xmlSecAppCmdLineTimeParamRead(const char* str, time_t* t, int is_gmt_time) {
     struct tm tm;
-    int n;
-    int consumed = 0;
-    const char* rest;
+    int year, month, day, hour, minute, second;
+    const char* p;
 
     if((str == NULL) || (t == NULL)) {
         return(-1);
     }
     memset(&tm, 0, sizeof(tm));
     tm.tm_isdst = -1;
-    n = XMLSEC_SCANF(str, "%4d-%2d-%2d%*c%2d:%2d:%2d%n",
-                        &tm.tm_year, &tm.tm_mon, &tm.tm_mday,
-                        &tm.tm_hour, &tm.tm_min, &tm.tm_sec, &consumed);
-    if(n != 6) {
+
+    /* Parse the documented "YYYY-MM-DD HH:MM:SS" format. The date and time
+     * parts are separated by a single character (any character, e.g. a space,
+     * 'T', or '+'), followed by optional whitespace. Parsing is done manually
+     * (rather than with sscanf("%n")) so that it behaves identically on MSVC,
+     * where sscanf_s does not support the %n specifier. */
+    p = str;
+
+    year = xmlSecAppCmdLineParseDigits(&p, 4);
+    if((year < 0) || (*p != '-')) {
         return(-1);
     }
+    p++;
 
-    /* reject trailing garbage (trailing whitespace is allowed) */
-    rest = str + consumed;
-    while(*rest != '\0') {
-        if((*rest != ' ') && (*rest != '\t')) {
-            return(-1);
-        }
-        ++rest;
+    month = xmlSecAppCmdLineParseDigits(&p, 2);
+    if((month < 0) || (*p != '-')) {
+        return(-1);
+    }
+    p++;
+
+    day = xmlSecAppCmdLineParseDigits(&p, 2);
+    if(day < 0) {
+        return(-1);
+    }
+
+    /* Skip exactly one separator character between the date and the time,
+     * matching the original sscanf "%*c" (which accepts any single character,
+     * e.g. a space, 'T', or '+'), then any additional leading whitespace. */
+    if(*p == '\0') {
+        return(-1);
+    }
+    p++;
+    while(isspace((unsigned char)*p)) {
+        p++;
+    }
+
+    hour = xmlSecAppCmdLineParseDigits(&p, 2);
+    if((hour < 0) || (*p != ':')) {
+        return(-1);
+    }
+    p++;
+
+    minute = xmlSecAppCmdLineParseDigits(&p, 2);
+    if((minute < 0) || (*p != ':')) {
+        return(-1);
+    }
+    p++;
+
+    second = xmlSecAppCmdLineParseDigits(&p, 2);
+    if(second < 0) {
+        return(-1);
+    }
+
+    /* reject trailing garbage (trailing whitespaces are allowed) */
+    while(isspace((unsigned char)*p)) {
+        p++;
+    }
+    if((*p) != '\0') {
+        return(-1);
     }
 
-    if((tm.tm_year < 1900)
-      || (tm.tm_mon  < 1) || (tm.tm_mon  > 12)
-      || (tm.tm_mday < 1) || (tm.tm_mday > 31)
-      || (tm.tm_hour < 0) || (tm.tm_hour > 23)
-      || (tm.tm_min  < 0) || (tm.tm_min  > 59)
-      || (tm.tm_sec  < 0) || (tm.tm_sec  > 61)) {
+    /* check values for basic validity */
+    if((year < 1900)
+      || (month  < 1) || (month  > 12)
+      || (day    < 1) || (day    > 31)
+      || (hour   < 0) || (hour   > 23)
+      || (minute < 0) || (minute > 59)
+      || (second < 0) || (second > 61)
+    ) {
         return(-1);
     }
 
     /* reject invalid calendar dates (e.g. "2020-02-30") that the field
      * range checks above would otherwise let mktime() silently normalize */
-    if(xmlSecAppIsValidDate(tm.tm_year, tm.tm_mon, tm.tm_mday) == 0) {
+    if(xmlSecAppIsValidDate(year, month, day) == 0) {
         return(-1);
     }
 
-    tm.tm_year -= 1900; /* tm relative format year */
-    tm.tm_mon  -= 1; /* tm relative format month */
+    tm.tm_year = year - 1900; /* tm relative format year */
+    tm.tm_mon  = month - 1; /* tm relative format month */
+    tm.tm_mday = day;
+    tm.tm_hour = hour;
+    tm.tm_min  = minute;
+    tm.tm_sec  = second;
 
     if(is_gmt_time != 0) {
         (*t) = XMLSEC_MKGMTIME(&tm);

apps/oss-fuzz/standalone_fuzz_runner.c

@@ -117,6 +117,15 @@ int main(int argc, const char **argv) {
             fuzzer_close_file(f);
             continue;
         }
+        /* On 32-bit builds the file offset is 64-bit but size_t is 32-bit;
+         * reject files that would not fit in size_t to avoid silent truncation
+         * of the input size. The sizeof() guard keeps the SIZE_MAX cast valid
+         * (it is only evaluated when the offset type is wider than size_t). */
+        if (sizeof(fuzzer_file_offset_t) > sizeof(size_t) && len > (fuzzer_file_offset_t)SIZE_MAX) {
+            FUZZER_ERROR("file is too large", argv[i]);
+            fuzzer_close_file(f);
+            continue;
+        }
         rewind(f);
 
         /* Allocate at least one byte so that zero-length inputs still get a

apps/oss-fuzz/xmlsec_dsig_verify_target.c

@@ -80,12 +80,16 @@ static int do_init(void) {
         return -1;
     }
     if (xmlSecCheckVersion() != 1) {
+        xmlSecShutdown();
         return -1;
     }
     if (xmlSecOpenSSLAppInit(NULL) < 0) {
+        xmlSecShutdown();
         return -1;
     }
     if (xmlSecOpenSSLInit() < 0) {
+        xmlSecOpenSSLAppShutdown();
+        xmlSecShutdown();
         return -1;
     }
 
@@ -96,11 +100,17 @@ static int do_init(void) {
     /* Build a keys manager once and load the fixed trusted cert into it. */
     g_mngr = xmlSecKeysMngrCreate();
     if (g_mngr == NULL) {
+        xmlSecOpenSSLShutdown();
+        xmlSecOpenSSLAppShutdown();
+        xmlSecShutdown();
         return -1;
     }
     if (xmlSecOpenSSLAppDefaultKeysMngrInit(g_mngr) < 0) {
         xmlSecKeysMngrDestroy(g_mngr);
         g_mngr = NULL;
+        xmlSecOpenSSLShutdown();
+        xmlSecOpenSSLAppShutdown();
+        xmlSecShutdown();
         return -1;
     }
     /* Re-assemble the PEM cert (with real newline characters) into a buffer. */

apps/oss-fuzz/xmlsec_keyinfo_target.c

@@ -74,6 +74,7 @@ static int do_init(void) {
         return -1;
     }
     if (xmlSecOpenSSLInit() < 0) {
+        xmlSecOpenSSLAppShutdown();
         xmlSecShutdown();
         return -1;
     }

apps/oss-fuzz/xmlsec_relationship_target.c

@@ -59,17 +59,27 @@ static int do_init(void) {
     return 0;
 }
 
-static xmlNodePtr find_relationships(xmlNodePtr node) {
+/* Maximum recursion depth for find_relationships(). libxml2 caps element
+ * nesting at xmlParserMaxDepth (256 in normal mode; this target does not use
+ * XML_PARSE_HUGE), so a valid input can never reach this bound. It exists as
+ * a defense-in-depth cap so the recursion cannot exhaust the stack even if
+ * the parser depth limit were raised or absent. */
+#define RELATIONSHIPS_MAX_DEPTH 10000
+
+static xmlNodePtr find_relationships(xmlNodePtr node, int depth) {
     xmlNodePtr cur;
 
+    if (depth >= RELATIONSHIPS_MAX_DEPTH) {
+        return NULL;
+    }
     for (cur = node; cur != NULL; cur = cur->next) {
         if (cur->type == XML_ELEMENT_NODE && cur->ns != NULL &&
             cur->ns->href != NULL &&
             xmlStrEqual(cur->ns->href, xmlSecRelationshipsNs)) {
             return cur;
         }
         if (cur->children != NULL) {
-            xmlNodePtr found = find_relationships(cur->children);
+            xmlNodePtr found = find_relationships(cur->children, depth + 1);
             if (found != NULL) {
                 return found;
             }
@@ -103,7 +113,7 @@ int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     }
 
     transform_node = xmlSecFindNode(root, xmlSecNodeTransform, xmlSecDSigNs);
-    relationships_node = find_relationships(root);
+    relationships_node = find_relationships(root, 0);
     if (transform_node == NULL || relationships_node == NULL) {
         goto done;
     }

apps/unit_tests/base64_unit_tests.c

@@ -98,17 +98,23 @@ test_base64_decode_exact_size(
     const char * expected,
     xmlSecSize expectedSize
 ) {
-    xmlSecByte decoded[256];
+    xmlSecByte decoded[257];
     xmlSecSize decodedSize = 0;
+    xmlSecSize i;
     int ret;
 
     xmlSecAssert(name != NULL);
     xmlSecAssert(str != NULL);
     xmlSecAssert(expected != NULL);
-    xmlSecAssert(expectedSize <= sizeof(decoded));
+    /* Keep at least one canary byte past the declared buffer size. */
+    xmlSecAssert(expectedSize < sizeof(decoded));
 
     testStart(name);
 
+    /* Fill the decode target with 0xAA and the remainder with a canary (0xBB)
+     * so any write past the declared buffer size (expectedSize) is detected,
+     * even though the backing array is larger. */
+    memset(decoded, 0xBB, sizeof(decoded));
     memset(decoded, 0xAA, expectedSize);
     ret = xmlSecBase64Decode_ex(BAD_CAST str, decoded, expectedSize, &decodedSize);
     if(ret < 0) {
@@ -118,6 +124,17 @@ test_base64_decode_exact_size(
         return;
     }
 
+    /* The function must not write past the declared buffer size; the canary
+     * bytes beyond expectedSize must remain untouched. */
+    for (i = expectedSize; i < sizeof(decoded); i++) {
+        if (decoded[i] != 0xBB) {
+            testLog("Error: base64 decode wrote past the exactly-sized buffer (%d bytes) for '%s'\n",
+                (int)expectedSize, str);
+            testFinishedFailure();
+            return;
+        }
+    }
+
     if(decodedSize != expectedSize) {
         testLog("Error: base64 decode returned size=%d (expected: %d) for '%s'\n",
             (int)decodedSize, (int)expectedSize, str);

apps/unit_tests/buffer_unit_tests.c

@@ -1489,6 +1489,7 @@ static void
 test_buffer_read_file(void) {
     xmlSecBufferPtr buf = NULL;
     char tmpName[160] = { '\0' };
+    char tmpNameMissing[160] = { '\0' };
     const xmlSecByte payload[] = { 0x00, 0x01, 0x02, 0xAB, 0xCD, 0xEF, 0xFF };
     FILE* f = NULL;
     int fileCreated = 0;
@@ -1501,6 +1502,13 @@ test_buffer_read_file(void) {
         testFinishedFailure();
         return;
     }
+    /* A name under the temp dir that is intentionally never created, for the
+     * missing-file negative test (avoids a CWD-relative hardcoded name). */
+    if(test_buffer_make_temp_name(tmpNameMissing, sizeof(tmpNameMissing), "readfile_missing.bin") < 0) {
+        testLog("Error: failed to build missing-file temp name\n");
+        testFinishedFailure();
+        return;
+    }
 
     /* write a known payload to a temp file in the temp directory */
 #ifndef _MSC_VER
@@ -1544,7 +1552,7 @@ test_buffer_read_file(void) {
     }
 
     /* a missing file must be rejected */
-    if(xmlSecBufferReadFile(buf, "xmlsec_unit_tests_no_such_file_xyz.bin") >= 0) {
+    if(xmlSecBufferReadFile(buf, tmpNameMissing) >= 0) {
         testLog("Error: xmlSecBufferReadFile should fail for a missing file\n");
         goto done;
     }

apps/unit_tests/list_unit_tests.c

@@ -1035,6 +1035,9 @@ test_ptr_list_invalid_alloc_mode(void) {
     ret = xmlSecPtrListAdd(list, item);
     if(ret >= 0) {
         testLog("Error: xmlSecPtrListAdd succeeded with an invalid allocation mode\n");
+        /* The add succeeded, so the item is now owned by the list; set item to
+         * NULL so the done: path does not destroy it a second time. */
+        item = NULL;
         goto done;
     }
     if(xmlSecPtrListGetSize(list) != 0) {

apps/unit_tests/transform_helpers_unit_tests.c

@@ -193,7 +193,8 @@ test_xmlSecTransformChaCha20ParamsWrite_roundtrip(void) {
     xmlFree(counterContent);
 
     ret = xmlSecTransformChaCha20ParamsRead(node, ivRoundTrip, sizeof(ivRoundTrip), &ivSize, &noncePresent);
-    if((ret < 0) || (ivSize != XMLSEC_CHACHA20_IV_SIZE) || (memcmp(ivRoundTrip, iv, sizeof(iv)) != 0)) {
+    if((ret < 0) || (ivSize != XMLSEC_CHACHA20_IV_SIZE) || (noncePresent != 1) ||
+       (memcmp(ivRoundTrip, iv, sizeof(iv)) != 0)) {
         testLog("Error: ChaCha20 params write did not round-trip through strict read\n");
         xmlFreeDoc(doc);
         testFinishedFailure();

apps/unit_tests/xmlsec_unit_tests.c

@@ -334,15 +334,15 @@ void testStart(const char * name) {
 
 void testFinishedSuccess(void) {
     if(g_testGroupSkip) { return; }
-    fprintf(stdout, "    %s     OK\n", testsName);
+    fprintf(stdout, "    %s     OK\n", testsName != NULL ? testsName : "(no test name)");
     testLogReset();
     testsFinishedSuccess += 1;
     testsName = NULL;
 }
 
 void testFinishedFailure(void) {
     if(g_testGroupSkip) { return; }
-    fprintf(stdout, "    %s     FAILED\n", testsName);
+    fprintf(stdout, "    %s     FAILED\n", testsName != NULL ? testsName : "(no test name)");
     testLogFlush();
     testsFinishedFailed += 1;
     testsName = NULL;