CVE-2021-34121: Out-of-bounds Read (CWE-125)
value: An Out of Bounds flaw was discovered in htmodoc 1.9.12 in function parse_tree() in toc.cxx, this possibly leads to memory layout information leaking in the data. This might be used in a chain of vulnerability in order to reach code execution.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 7.8
Published: 2023-07-18T14:15Z
Repository: https://github.com/michaelrsweet/htmldoc
Language: Other
Files changed: CHANGES.md, htmldoc/toc.cxx, htmldoc/util.cxx
Commit date: 2021-05-27 08:58:21 -0400
Commit message: Fix array overflow for headings using roman numerals (Issue #433)

Diff: diff --git a/CHANGES.md b/CHANGES.md
index 39994805..bc8ad526 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,5 +1,11 @@
+# Changes in HTMLDOC v1.9.13
+
+- Fixed an issue with large values for roman numerals and letters in headings
+  (Issue #433)
+
+
 # Changes in HTMLDOC v1.9.12
 
 - Fixed a crash bug with "data:" URIs and EPUB output (Issue #410)
 - Fixed crash bugs for books (Issue #412, Issue #414)
 - Fixed a number-up crash bug (Issue #413)
diff --git a/htmldoc/toc.cxx b/htmldoc/toc.cxx
index b9e7b68b..2c27f099 100644
--- a/htmldoc/toc.cxx
+++ b/htmldoc/toc.cxx
@@ -120,40 +120,10 @@ parse_tree(tree_t *t)		/* I - Document tree */
 		link[255],	/* Actual link */
 		baselink[255],	/* Base link (numbered) */
 		*existing;	/* Existing link string */
   int		i, level;	/* Header level */
   uchar		*var;		/* Starting value/type for this level */
-  static const char *ones[10] =
-		{
-		  "",	"i",	"ii",	"iii",	"iv",
-		  "v",	"vi",	"vii",	"viii",	"ix"
-		},
-		*tens[10] =
-		{
-		  "",	"x",	"xx",	"xxx",	"xl",
-		  "l",	"lx",	"lxx",	"lxxx",	"xc"
-		},
-		*hundreds[10] =
-		{
-		  "",	"c",	"cc",	"ccc",	"cd",
-		  "d",	"dc",	"dcc",	"dccc",	"cm"
-		},
-		*ONES[10] =
-		{
-		  "",	"I",	"II",	"III",	"IV",
-		  "V",	"VI",	"VII",	"VIII",	"IX"
-		},
-		*TENS[10] =
-		{
-		  "",	"X",	"XX",	"XXX",	"XL",
-		  "L",	"LX",	"LXX",	"LXXX",	"XC"
-		},
-		*HUNDREDS[10] =
-		{
-		  "",	"C",	"CC",	"CCC",	"CD",
-		  "D",	"DC",	"DCC",	"DCCC",	"CM"
-		};
 
 
   while (t != NULL)
   {
     switch (t->markup)
@@ -210,34 +180,11 @@ parse_tree(tree_t *t)		/* I - Document tree */
             if (i == 0)
               snprintf((char *)baseptr, sizeof(baselink) - (size_t)(baseptr - baselink), "%d", TocDocCount);
             else
               snprintf((char *)baseptr, sizeof(baselink) - (size_t)(baseptr - baselink), "%d", heading_numbers[i]);
 
-            switch (heading_types[i])
-            {
-              case '1' :
-                  snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%d", heading_numbers[i]);
-                  break;
-              case 'a' :
-                  if (heading_numbers[i] > 26)
-                    snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%c%c", 'a' + (heading_numbers[i] / 26) - 1, 'a' + (heading_numbers[i] % 26) - 1);
-                  else
-                    snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%c", 'a' + heading_numbers[i] - 1);
-                  break;
-              case 'A' :
-                  if (heading_numbers[i] > 26)
-                    snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%c%c", 'A' + (heading_numbers[i] / 26) - 1, 'A' + (heading_numbers[i] % 26) - 1);
-                  else
-                    snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%c", 'A' + heading_numbers[i] - 1);
-                  break;
-              case 'i' :
-                  snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%s%s%s", hundreds[heading_numbers[i] / 100], tens[(heading_numbers[i] / 10) % 10], ones[heading_numbers[i] % 10]);
-                  break;
-              case 'I' :
-                  snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%s%s%s", HUNDREDS[heading_numbers[i] / 100], TENS[(heading_numbers[i] / 10) % 10], ONES[heading_numbers[i] % 10]);
-                  break;
-            }
+            strlcpy((char *)headptr, format_number(heading_numbers[i], heading_types[i]), sizeof(heading) - (size_t)(headptr - heading));
 
             if (i < level)
             {
               strlcat((char *)heading, ".", sizeof(heading));
               strlcat((char *)baselink, "_", sizeof(baselink));
diff --git a/htmldoc/util.cxx b/htmldoc/util.cxx
index 3d2c6b83..4d63e5b3 100644
--- a/htmldoc/util.cxx
+++ b/htmldoc/util.cxx
@@ -33,14 +33,18 @@ format_number(int  n,		/* I - Number */
 		*tens[10] =	/* Roman numerals, 10-90 */
 		{
 		  "",	"x",	"xx",	"xxx",	"xl",
 		  "l",	"lx",	"lxx",	"lxxx",	"xc"
 		},
-		*hundreds[10] =	/* Roman numerals, 100-900 */
+		*hundreds[30] =	/* Roman numerals, 100-2900 */
 		{
-		  "",	"c",	"cc",	"ccc",	"cd",
-		  "d",	"dc",	"dcc",	"dccc",	"cm"
+		  "",    "c",    "cc",    "ccc",    "cd",
+		  "d",   "dc",   "dcc",   "dccc",   "cm",
+		  "m",   "mc",   "cc",    "ccc",    "cd",
+		  "m",   "mdc",  "mdcc",  "mdccc",  "mcm",
+		  "mm",  "mmc",  "mmcc",  "mmccc",  "mmcd",
+		  "mmd", "mmdc", "mmdcc", "mmdccc", "mmcm"
 		};
   static const char *ONES[10] =	/* Roman numerals, 0-9 */
 		{
 		  "",	"I",	"II",	"III",	"IV",
 		  "V",	"VI",	"VII",	"VIII",	"IX"
@@ -48,14 +52,18 @@ format_number(int  n,		/* I - Number */
 		*TENS[10] =	/* Roman numerals, 10-90 */
 		{
 		  "",	"X",	"XX",	"XXX",	"XL",
 		  "L",	"LX",	"LXX",	"LXXX",	"XC"
 		},
-		*HUNDREDS[10] =	/* Roman numerals, 100-900 */
+		*HUNDREDS[30] =	/* Roman numerals, 100-2900 */
 		{
-		  "",	"C",	"CC",	"CCC",	"CD",
-		  "D",	"DC",	"DCC",	"DCCC",	"CM"
+		  "",    "C",    "CC",    "CCC",    "CD",
+		  "D",   "DC",   "DCC",   "DCCC",   "CM",
+		  "M",   "MC",   "CC",    "CCC",    "CD",
+		  "M",   "MDC",  "MDCC",  "MDCCC",  "MCM",
+		  "MM",  "MMC",  "MMCC",  "MMCCC",  "MMCD",
+		  "MMD", "MMDC", "MMDCC", "MMDCCC", "MMCM"
 		};
   static char	buffer[1024];	/* String buffer */
 
 
   switch (f)
@@ -63,43 +71,45 @@ format_number(int  n,		/* I - Number */
     default :
         buffer[0] = '\0';
 	break;
 
     case 'a' :
-        if (n >= (26 * 26))
-	  buffer[0] = '\0';
-        else if (n > 26)
+        if (n > (26 * 26))
+          n = (n % (26 * 26)) + 1;
+
+        if (n > 26)
           snprintf(buffer, sizeof(buffer), "%c%c", 'a' + (n / 26) - 1, 'a' + (n % 26) - 1);
         else
           snprintf(buffer, sizeof(buffer), "%c", 'a' + n - 1);
         break;
 
     case 'A' :
-        if (n >= (26 * 26))
-	  buffer[0] = '\0';
-        else if (n > 26)
+        if (n > (26 * 26))
+          n = (n % (26 * 26)) + 1;
+
+        if (n > 26)
           snprintf(buffer, sizeof(buffer), "%c%c", 'A' + (n / 26) - 1, 'A' + (n % 26) - 1);
         else
           snprintf(buffer, sizeof(buffer), "%c", 'A' + n - 1);
         break;
 
     case '1' :
         snprintf(buffer, sizeof(buffer), "%d", n);
         break;
 
     case 'i' :
-        if (n >= 1000)
-	  buffer[0] = '\0';
-	else
-          snprintf(buffer, sizeof(buffer), "%s%s%s", hundreds[n / 100], tens[(n / 10) % 10], ones[n % 10]);
+        if (n >= 3000)
+          n = (n % 3000) + 1;
+        
+	snprintf(buffer, sizeof(buffer), "%s%s%s", hundreds[n / 100], tens[(n / 10) % 10], ones[n % 10]);
         break;
 
     case 'I' :
-        if (n >= 1000)
-	  buffer[0] = '\0';
-	else
-          snprintf(buffer, sizeof(buffer), "%s%s%s", HUNDREDS[n / 100], TENS[(n / 10) % 10], ONES[n % 10]);
+        if (n >= 3000)
+          n = (n % 3000) + 1;
+        
+	snprintf(buffer, sizeof(buffer), "%s%s%s", HUNDREDS[n / 100], TENS[(n / 10) % 10], ONES[n % 10]);
         break;
   }
 
   return (buffer);
 }

Vulnerable code:   static const char *ones[10] =
		{
		  "",	"i",	"ii",	"iii",	"iv",
		  "v",	"vi",	"vii",	"viii",	"ix"
		},
		*tens[10] =
		{
		  "",	"x",	"xx",	"xxx",	"xl",
		  "l",	"lx",	"lxx",	"lxxx",	"xc"
		},
		*hundreds[10] =
		{
		  "",	"c",	"cc",	"ccc",	"cd",
		  "d",	"dc",	"dcc",	"dccc",	"cm"
		},
		*ONES[10] =
		{
		  "",	"I",	"II",	"III",	"IV",
		  "V",	"VI",	"VII",	"VIII",	"IX"
		},
		*TENS[10] =
		{
		  "",	"X",	"XX",	"XXX",	"XL",
		  "L",	"LX",	"LXX",	"LXXX",	"XC"
		},
		*HUNDREDS[10] =
		{
		  "",	"C",	"CC",	"CCC",	"CD",
		  "D",	"DC",	"DCC",	"DCCC",	"CM"
		};
            switch (heading_types[i])
            {
              case '1' :
                  snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%d", heading_numbers[i]);
                  break;
              case 'a' :
                  if (heading_numbers[i] > 26)
                    snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%c%c", 'a' + (heading_numbers[i] / 26) - 1, 'a' + (heading_numbers[i] % 26) - 1);
                  else
                    snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%c", 'a' + heading_numbers[i] - 1);
                  break;
              case 'A' :
                  if (heading_numbers[i] > 26)
                    snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%c%c", 'A' + (heading_numbers[i] / 26) - 1, 'A' + (heading_numbers[i] % 26) - 1);
                  else
                    snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%c", 'A' + heading_numbers[i] - 1);
                  break;
              case 'i' :
                  snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%s%s%s", hundreds[heading_numbers[i] / 100], tens[(heading_numbers[i] / 10) % 10], ones[heading_numbers[i] % 10]);
                  break;
              case 'I' :
                  snprintf((char *)headptr, sizeof(heading) - (size_t)(headptr - heading), "%s%s%s", HUNDREDS[heading_numbers[i] / 100], TENS[(heading_numbers[i] / 10) % 10], ONES[heading_numbers[i] % 10]);
                  break;
            }
		*hundreds[10] =	/* Roman numerals, 100-900 */
		  "",	"c",	"cc",	"ccc",	"cd",
		  "d",	"dc",	"dcc",	"dccc",	"cm"
		*HUNDREDS[10] =	/* Roman numerals, 100-900 */
		  "",	"C",	"CC",	"CCC",	"CD",
		  "D",	"DC",	"DCC",	"DCCC",	"CM"
        if (n >= (26 * 26))
	  buffer[0] = '\0';
        else if (n > 26)
        if (n >= (26 * 26))
	  buffer[0] = '\0';
        else if (n > 26)
        if (n >= 1000)
	  buffer[0] = '\0';
	else
          snprintf(buffer, sizeof(buffer), "%s%s%s", hundreds[n / 100], tens[(n / 10) % 10], ones[n % 10]);
        if (n >= 1000)
	  buffer[0] = '\0';
	else
          snprintf(buffer, sizeof(buffer), "%s%s%s", HUNDREDS[n / 100], TENS[(n / 10) % 10], ONES[n % 10]);
Fixed code: # Changes in HTMLDOC v1.9.13

- Fixed an issue with large values for roman numerals and letters in headings
  (Issue #433)


            strlcpy((char *)headptr, format_number(heading_numbers[i], heading_types[i]), sizeof(heading) - (size_t)(headptr - heading));
		*hundreds[30] =	/* Roman numerals, 100-2900 */
		  "",    "c",    "cc",    "ccc",    "cd",
		  "d",   "dc",   "dcc",   "dccc",   "cm",
		  "m",   "mc",   "cc",    "ccc",    "cd",
		  "m",   "mdc",  "mdcc",  "mdccc",  "mcm",
		  "mm",  "mmc",  "mmcc",  "mmccc",  "mmcd",
		  "mmd", "mmdc", "mmdcc", "mmdccc", "mmcm"
		*HUNDREDS[30] =	/* Roman numerals, 100-2900 */
		  "",    "C",    "CC",    "CCC",    "CD",
		  "D",   "DC",   "DCC",   "DCCC",   "CM",
		  "M",   "MC",   "CC",    "CCC",    "CD",
		  "M",   "MDC",  "MDCC",  "MDCCC",  "MCM",
		  "MM",  "MMC",  "MMCC",  "MMCCC",  "MMCD",
		  "MMD", "MMDC", "MMDCC", "MMDCCC", "MMCM"
        if (n > (26 * 26))
          n = (n % (26 * 26)) + 1;

        if (n > 26)
        if (n > (26 * 26))
          n = (n % (26 * 26)) + 1;

        if (n > 26)
        if (n >= 3000)
          n = (n % 3000) + 1;
        
	snprintf(buffer, sizeof(buffer), "%s%s%s", hundreds[n / 100], tens[(n / 10) % 10], ones[n % 10]);
        if (n >= 3000)
          n = (n % 3000) + 1;
        
	snprintf(buffer, sizeof(buffer), "%s%s%s", HUNDREDS[n / 100], TENS[(n / 10) % 10], ONES[n % 10]);