From 7687d18f3fcd050c3f81cc04386da3a4431532c9 Mon Sep 17 00:00:00 2001 From: Diogo Santos <170260504+S4N-T0S@users.noreply.github.com> Date: Tue, 5 May 2026 11:54:17 +0100 Subject: [PATCH 1/4] Off-By-One Null Byte Fix --- src/Common/Xml.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Common/Xml.c b/src/Common/Xml.c index 357b7de183..6105070be9 100644 --- a/src/Common/Xml.c +++ b/src/Common/Xml.c @@ -113,7 +113,7 @@ char *XmlGetAttributeText (char *xmlNode, const char *xmlAttrName, char *xmlAttr t = ((char*)strchr (t, '"')) + 1; e = strchr (t, '"'); l = (int)(e - t); - if (e == NULL || l > xmlAttrValueSize) return NULL; + if (e == NULL || l >= xmlAttrValueSize) return NULL; memcpy (xmlAttrValue, t, l); xmlAttrValue[l] = 0; @@ -141,7 +141,7 @@ char *XmlGetNodeText (char *xmlNode, char *xmlText, int xmlTextSize) if (e == NULL) return NULL; l = (int)(e - t); - if (e == NULL || l > xmlTextSize) return NULL; + if (e == NULL || l >= xmlTextSize) return NULL; while (i < l) { From de59d76d109b56ac56e6fc152fa224f7558c9c4d Mon Sep 17 00:00:00 2001 From: Diogo Santos <170260504+S4N-T0S@users.noreply.github.com> Date: Tue, 5 May 2026 16:05:15 +0100 Subject: [PATCH 2/4] Add XML parser tests and improve XmlGetAttributeText handling --- src/Common/Tests.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++ src/Common/Tests.h | 1 + src/Common/Xml.c | 35 ++++++++++++++++++-------- 3 files changed, 88 insertions(+), 10 deletions(-) diff --git a/src/Common/Tests.c b/src/Common/Tests.c index 2ac8d8ffa1..af9ec76dc7 100644 --- a/src/Common/Tests.c +++ b/src/Common/Tests.c @@ -20,6 +20,7 @@ #include #include "Pkcs5.h" #include "cpu.h" +#include "Xml.h" typedef struct { CRYPTOPP_ALIGN_DATA(16) unsigned __int8 key1[32]; @@ -1447,6 +1448,10 @@ static BOOL DoAutoTestAlgorithms (void) if (!TestSectorBufEncryption (ci)) bFailed = TRUE; + /* XML parser tests */ + if (!XmlTest ()) + bFailed = TRUE; + crypto_close (ci); return !bFailed; } @@ -1744,3 +1749,60 @@ BOOL test_pkcs5 () #endif return TRUE; } + +BOOL XmlTest (void) +{ + char buffer[10]; + + /* XmlGetAttributeText tests */ + + /* 1. length size - 1 accepted */ + char xmlAttrValid[] = ""; + if (XmlGetAttributeText (xmlAttrValid, "attr", buffer, sizeof (buffer)) == NULL + || strcmp (buffer, "123456789") != 0) + return FALSE; + + /* 2. length size rejected (off-by-one: would write NUL past buffer end) */ + char xmlAttrOverflow[] = ""; + if (XmlGetAttributeText (xmlAttrOverflow, "attr", buffer, sizeof (buffer)) != NULL) + return FALSE; + + /* 3. malformed: closing quote absent returns NULL */ + char xmlAttrMissingQuote[] = ""; + if (XmlGetAttributeText (xmlAttrMissingQuote, "attr", buffer, sizeof (buffer)) != NULL) + return FALSE; + + /* 4. closing quote belongs to a later tag, not the current one */ + char xmlAttrCrossTag[] = ""; + if (XmlGetAttributeText (xmlAttrCrossTag, "attr", buffer, sizeof (buffer)) != NULL) + return FALSE; + + + /* XmlGetNodeText tests */ + + /* 5. length size - 1 accepted */ + char xmlNodeValid[] = "123456789"; + if (XmlGetNodeText (xmlNodeValid, buffer, sizeof (buffer)) == NULL + || strcmp (buffer, "123456789") != 0) + return FALSE; + + /* 6. length size rejected (off-by-one: would write NUL past buffer end) */ + char xmlNodeOverflow[] = "1234567890"; + if (XmlGetNodeText (xmlNodeOverflow, buffer, sizeof (buffer)) != NULL) + return FALSE; + + /* 7. escaped text accepted: raw input is larger than buffer but decoded + output fits. Decoded: "<>&456789" (9 chars), buffer is 10 bytes. */ + char xmlNodeEscaped[] = "<>&456789"; + if (XmlGetNodeText (xmlNodeEscaped, buffer, sizeof (buffer)) == NULL + || strcmp (buffer, "<>&456789") != 0) + return FALSE; + + /* 8. escaped text rejected: decoded output is exactly size (10 chars), + leaving no room for the NUL terminator. Decoded: "<>&4567890" (10 chars). */ + char xmlNodeEscapedOverflow[] = "<>&4567890"; + if (XmlGetNodeText (xmlNodeEscapedOverflow, buffer, sizeof (buffer)) != NULL) + return FALSE; + + return TRUE; +} diff --git a/src/Common/Tests.h b/src/Common/Tests.h index 60fab0b680..c4673c5e4d 100644 --- a/src/Common/Tests.h +++ b/src/Common/Tests.h @@ -25,6 +25,7 @@ BOOL test_pkcs5 (void); BOOL TestSectorBufEncryption (); BOOL TestLegacySectorBufEncryption (); BOOL AutoTestAlgorithms (void); +BOOL XmlTest (void); #ifdef __cplusplus } diff --git a/src/Common/Xml.c b/src/Common/Xml.c index 6105070be9..22f42b5201 100644 --- a/src/Common/Xml.c +++ b/src/Common/Xml.c @@ -84,16 +84,20 @@ char *XmlFindElementByAttributeValue (char *xml, char *nodeName, const char *att char *XmlGetAttributeText (char *xmlNode, const char *xmlAttrName, char *xmlAttrValue, int xmlAttrValueSize) { char *t = xmlNode; - char *e = xmlNode; + char *nodeEnd = xmlNode; + char *quote1, *quote2; int l = 0; + if (xmlAttrValueSize <= 0) + return NULL; + xmlAttrValue[0] = 0; if (t[0] != '<') return NULL; - e = strchr (e, '>'); - if (e == NULL) return NULL; + nodeEnd = strchr (nodeEnd, '>'); + if (nodeEnd == NULL) return NULL; - while ((t = strstr (t, xmlAttrName)) && t < e) + while ((t = strstr (t, xmlAttrName)) && t < nodeEnd) { char *o = t + strlen (xmlAttrName); if (t[-1] == ' ' @@ -108,12 +112,17 @@ char *XmlGetAttributeText (char *xmlNode, const char *xmlAttrName, char *xmlAttr t++; } - if (t == NULL || t > e) return NULL; + if (t == NULL || t > nodeEnd) return NULL; - t = ((char*)strchr (t, '"')) + 1; - e = strchr (t, '"'); - l = (int)(e - t); - if (e == NULL || l >= xmlAttrValueSize) return NULL; + quote1 = strchr (t, '"'); + if (quote1 == NULL || quote1 > nodeEnd) return NULL; + t = quote1 + 1; + + quote2 = strchr (t, '"'); + if (quote2 == NULL || quote2 > nodeEnd) return NULL; + + l = (int)(quote2 - t); + if (l < 0 || l >= xmlAttrValueSize) return NULL; memcpy (xmlAttrValue, t, l); xmlAttrValue[l] = 0; @@ -128,6 +137,9 @@ char *XmlGetNodeText (char *xmlNode, char *xmlText, int xmlTextSize) char *e = xmlNode + 1; int l = 0, i = 0, j = 0; + if (xmlTextSize <= 0) + return NULL; + xmlText[0] = 0; if (t[0] != '<') @@ -141,10 +153,13 @@ char *XmlGetNodeText (char *xmlNode, char *xmlText, int xmlTextSize) if (e == NULL) return NULL; l = (int)(e - t); - if (e == NULL || l >= xmlTextSize) return NULL; + if (l < 0) return NULL; while (i < l) { + if (j >= xmlTextSize - 1) + return NULL; + if (BeginsWith (&t[i], "<")) { xmlText[j++] = '<'; From 97d73a1e4b0d1cee95d6719deb64c9a113591bca Mon Sep 17 00:00:00 2001 From: Diogo Santos <170260504+S4N-T0S@users.noreply.github.com> Date: Sat, 9 May 2026 19:11:39 +0100 Subject: [PATCH 3/4] Refactor XML testing: integrate XmlTest into AutoTestAlgorithms, add sentinel test for XmlGetNodeText insuficient output size. --- src/Common/Dlgcode.c | 8 ++++- src/Common/Tests.c | 61 --------------------------------------- src/Common/Tests.h | 1 - src/Common/Xml.c | 69 ++++++++++++++++++++++++++++++++++++++++++++ src/Common/Xml.h | 4 +++ 5 files changed, 80 insertions(+), 63 deletions(-) diff --git a/src/Common/Dlgcode.c b/src/Common/Dlgcode.c index 33fedf609b..269b2bda2e 100644 --- a/src/Common/Dlgcode.c +++ b/src/Common/Dlgcode.c @@ -7705,8 +7705,14 @@ CipherTestDialogProc (HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam) if (lw == IDC_AUTO) { + BOOL testsPassed; WaitCursor (); - if (!AutoTestAlgorithms()) + testsPassed = AutoTestAlgorithms(); + #if !defined(TC_WINDOWS_DRIVER) && !defined(_UEFI) + if (testsPassed && !XmlTest()) + testsPassed = FALSE; + #endif + if (!testsPassed) { ShowWindow(GetDlgItem(hwndDlg, IDC_TESTS_MESSAGE), SW_SHOWNORMAL); SetWindowTextW(GetDlgItem(hwndDlg, IDC_TESTS_MESSAGE), GetString ("TESTS_FAILED")); diff --git a/src/Common/Tests.c b/src/Common/Tests.c index af9ec76dc7..955fc5fe3f 100644 --- a/src/Common/Tests.c +++ b/src/Common/Tests.c @@ -20,7 +20,6 @@ #include #include "Pkcs5.h" #include "cpu.h" -#include "Xml.h" typedef struct { CRYPTOPP_ALIGN_DATA(16) unsigned __int8 key1[32]; @@ -1448,10 +1447,6 @@ static BOOL DoAutoTestAlgorithms (void) if (!TestSectorBufEncryption (ci)) bFailed = TRUE; - /* XML parser tests */ - if (!XmlTest ()) - bFailed = TRUE; - crypto_close (ci); return !bFailed; } @@ -1750,59 +1745,3 @@ BOOL test_pkcs5 () return TRUE; } -BOOL XmlTest (void) -{ - char buffer[10]; - - /* XmlGetAttributeText tests */ - - /* 1. length size - 1 accepted */ - char xmlAttrValid[] = ""; - if (XmlGetAttributeText (xmlAttrValid, "attr", buffer, sizeof (buffer)) == NULL - || strcmp (buffer, "123456789") != 0) - return FALSE; - - /* 2. length size rejected (off-by-one: would write NUL past buffer end) */ - char xmlAttrOverflow[] = ""; - if (XmlGetAttributeText (xmlAttrOverflow, "attr", buffer, sizeof (buffer)) != NULL) - return FALSE; - - /* 3. malformed: closing quote absent returns NULL */ - char xmlAttrMissingQuote[] = ""; - if (XmlGetAttributeText (xmlAttrMissingQuote, "attr", buffer, sizeof (buffer)) != NULL) - return FALSE; - - /* 4. closing quote belongs to a later tag, not the current one */ - char xmlAttrCrossTag[] = ""; - if (XmlGetAttributeText (xmlAttrCrossTag, "attr", buffer, sizeof (buffer)) != NULL) - return FALSE; - - - /* XmlGetNodeText tests */ - - /* 5. length size - 1 accepted */ - char xmlNodeValid[] = "123456789"; - if (XmlGetNodeText (xmlNodeValid, buffer, sizeof (buffer)) == NULL - || strcmp (buffer, "123456789") != 0) - return FALSE; - - /* 6. length size rejected (off-by-one: would write NUL past buffer end) */ - char xmlNodeOverflow[] = "1234567890"; - if (XmlGetNodeText (xmlNodeOverflow, buffer, sizeof (buffer)) != NULL) - return FALSE; - - /* 7. escaped text accepted: raw input is larger than buffer but decoded - output fits. Decoded: "<>&456789" (9 chars), buffer is 10 bytes. */ - char xmlNodeEscaped[] = "<>&456789"; - if (XmlGetNodeText (xmlNodeEscaped, buffer, sizeof (buffer)) == NULL - || strcmp (buffer, "<>&456789") != 0) - return FALSE; - - /* 8. escaped text rejected: decoded output is exactly size (10 chars), - leaving no room for the NUL terminator. Decoded: "<>&4567890" (10 chars). */ - char xmlNodeEscapedOverflow[] = "<>&4567890"; - if (XmlGetNodeText (xmlNodeEscapedOverflow, buffer, sizeof (buffer)) != NULL) - return FALSE; - - return TRUE; -} diff --git a/src/Common/Tests.h b/src/Common/Tests.h index c4673c5e4d..60fab0b680 100644 --- a/src/Common/Tests.h +++ b/src/Common/Tests.h @@ -25,7 +25,6 @@ BOOL test_pkcs5 (void); BOOL TestSectorBufEncryption (); BOOL TestLegacySectorBufEncryption (); BOOL AutoTestAlgorithms (void); -BOOL XmlTest (void); #ifdef __cplusplus } diff --git a/src/Common/Xml.c b/src/Common/Xml.c index 22f42b5201..a84bfa935f 100644 --- a/src/Common/Xml.c +++ b/src/Common/Xml.c @@ -158,7 +158,10 @@ char *XmlGetNodeText (char *xmlNode, char *xmlText, int xmlTextSize) while (i < l) { if (j >= xmlTextSize - 1) + { + xmlText[0] = 0; return NULL; + } if (BeginsWith (&t[i], "<")) { @@ -297,3 +300,69 @@ int XmlWriteFooter (FILE *file) return fputws (L"\n", file); } #endif !defined(_UEFI) + +#if !defined(TC_WINDOWS_DRIVER) && !defined(_UEFI) +BOOL XmlTest (void) +{ + char buffer[10]; + + /* XmlGetAttributeText tests */ + + /* 1. length size - 1 accepted */ + char xmlAttrValid[] = ""; + if (XmlGetAttributeText (xmlAttrValid, "attr", buffer, sizeof (buffer)) == NULL + || strcmp (buffer, "123456789") != 0) + return FALSE; + + /* 2. length size rejected (off-by-one: would write NUL past buffer end) */ + char xmlAttrOverflow[] = ""; + if (XmlGetAttributeText (xmlAttrOverflow, "attr", buffer, sizeof (buffer)) != NULL) + return FALSE; + + /* 3. malformed: closing quote absent returns NULL */ + char xmlAttrMissingQuote[] = ""; + if (XmlGetAttributeText (xmlAttrMissingQuote, "attr", buffer, sizeof (buffer)) != NULL) + return FALSE; + + /* 4. closing quote belongs to a later tag, not the current one */ + char xmlAttrCrossTag[] = ""; + if (XmlGetAttributeText (xmlAttrCrossTag, "attr", buffer, sizeof (buffer)) != NULL) + return FALSE; + + + /* XmlGetNodeText tests */ + + /* 5. length size - 1 accepted */ + char xmlNodeValid[] = "123456789"; + if (XmlGetNodeText (xmlNodeValid, buffer, sizeof (buffer)) == NULL + || strcmp (buffer, "123456789") != 0) + return FALSE; + + /* 6. length size rejected (off-by-one: would write NUL past buffer end) */ + char xmlNodeOverflow[] = "1234567890"; + if (XmlGetNodeText (xmlNodeOverflow, buffer, sizeof (buffer)) != NULL) + return FALSE; + + /* 7. escaped text accepted: raw input is larger than buffer but decoded + output fits. Decoded: "<>&456789" (9 chars), buffer is 10 bytes. */ + char xmlNodeEscaped[] = "<>&456789"; + if (XmlGetNodeText (xmlNodeEscaped, buffer, sizeof (buffer)) == NULL + || strcmp (buffer, "<>&456789") != 0) + return FALSE; + + /* 8. escaped text rejected: decoded output is exactly size (10 chars), + leaving no room for the NUL terminator. Decoded: "<>&4567890" (10 chars). */ + char xmlNodeEscapedOverflow[] = "<>&4567890"; + if (XmlGetNodeText (xmlNodeEscapedOverflow, buffer, sizeof (buffer)) != NULL) + return FALSE; + + /* 9. seed the buffer and verify overflow failure leaves it empty */ + char xmlNodeOverflowSeed[] = "1234567890"; + buffer[0] = 's'; + buffer[1] = 0; + if (XmlGetNodeText (xmlNodeOverflowSeed, buffer, sizeof (buffer)) != NULL || buffer[0] != 0) + return FALSE; + + return TRUE; +} +#endif diff --git a/src/Common/Xml.h b/src/Common/Xml.h index 6a1daded7b..cc3d7b2f13 100644 --- a/src/Common/Xml.h +++ b/src/Common/Xml.h @@ -21,6 +21,10 @@ char *XmlGetNodeText (char *xmlNode, char *xmlText, int xmlTextSize); char *XmlFindElementByAttributeValue (char *xml, char *nodeName, const char *attrName, const char *attrValue); char *XmlQuoteText (const char *textSrc, char *textDst, int textDstMaxSize); +#if !defined(TC_WINDOWS_DRIVER) && !defined(_UEFI) +BOOL XmlTest (void); +#endif + #if !defined(_UEFI) wchar_t *XmlQuoteTextW(const wchar_t *textSrc, wchar_t *textDst, int textDstMaxSize); int XmlWriteHeader (FILE *file); From 0a3ddcf4bba20e00b74898deeb22ecc8c92adfde Mon Sep 17 00:00:00 2001 From: Mounir IDRASSI Date: Sun, 10 May 2026 10:02:52 +0900 Subject: [PATCH 4/4] Remove no-op Tests.c change --- src/Common/Tests.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Common/Tests.c b/src/Common/Tests.c index 955fc5fe3f..2ac8d8ffa1 100644 --- a/src/Common/Tests.c +++ b/src/Common/Tests.c @@ -1744,4 +1744,3 @@ BOOL test_pkcs5 () #endif return TRUE; } -