switch_xml_decode: avoid NUL injection

strtol can parse negative values which opens the hole for a NUL
injection. The (invalid) entity "&#-256;" is parsed as 0xFFFFFF00 which
(when casted to a char) becomes 0.

Avoid this attack by using unsigned long integers. To avoid undefined
behavior due to negative shifts, restrict the upper bound of the code
points to the UTF-8 limits. (Add an assertion to make the Clang static
analyzer happy.)

Note: due to the specification of strtol, leading spaces and minus/plus
signs are also allowed, explicitly check for an integer. "&#0x1;" is
still accepted, but that is considered a minor issue.

diff --git a/src/switch_xml.c b/src/switch_xml.c
index 1173eb0155b21788d559153c892e80fa633279a1..50118720e0f0eb078133cd518f5f422f279a2c3e 100644 (file)
--- a/src/switch_xml.c
+++ b/src/switch_xml.c
@@ -538,7 +538,7 @@ static switch_xml_t switch_xml_err(switch_xml_root_t root, char *s, const char *
 static char *switch_xml_decode(char *s, char **ent, char t)
 {
        char *e, *r = s, *m = s;
-       long b, c, d, l;
+       unsigned long b, c, d, l;
 
        for (; *s; s++) {                       /* normalize line endings */
                while (*s == '\r') {
@@ -555,10 +555,17 @@ static char *switch_xml_decode(char *s, char **ent, char t)
                if (!*s)
                        break;
                else if (t != 'c' && !strncmp(s, "&#", 2)) {    /* character reference */
-                       if (s[2] == 'x')
-                               c = strtol(s + 3, &e, 16);      /* base 16 */
-                       else
-                               c = strtol(s + 2, &e, 10);      /* base 10 */
+                       char *code = s + 2;
+                       int base = 10;
+                       if (*code == 'x') {
+                               code++;
+                               base = 16;
+                       }
+                       if (!isxdigit((int)*code)) { /* "&# 1;" and "&#-1;" are invalid */
+                               s++;
+                               continue;
+                       }
+                       c = strtoul(code, &e, base);
                        if (!c || *e != ';') {
                                s++;
                                continue;
@@ -566,10 +573,14 @@ static char *switch_xml_decode(char *s, char **ent, char t)
                        /* not a character ref */
                        if (c < 0x80)
                                *(s++) = (char) c;      /* US-ASCII subset */
-                       else {                          /* multi-byte UTF-8 sequence */
+                       else if (c > 0x7FFFFFFF) { /* out of UTF-8 range */
+                               s++;
+                               continue;
+                       } else {                                /* multi-byte UTF-8 sequence */
                                for (b = 0, d = c; d; d /= 2)
                                        b++;            /* number of bits in c */
                                b = (b - 2) / 5;        /* number of bytes in payload */
+                               assert(b < 7);          /* because c <= 0x7FFFFFFF */
                                *(s++) = (char) ((0xFF << (7 - b)) | (c >> (6 * b)));   /* head */
                                while (b)
                                        *(s++) = (char) (0x80 | ((c >> (6 * --b)) & 0x3F));     /* payload */
@@ -580,8 +591,8 @@ static char *switch_xml_decode(char *s, char **ent, char t)
                        for (b = 0; ent[b] && strncmp(s + 1, ent[b], strlen(ent[b])); b += 2);  /* find entity in entity list */
 
                        if (ent[b++]) {         /* found a match */
-                               if ((c = (long) strlen(ent[b])) - 1 > (e = strchr(s, ';')) - s) {
-                                       l = (d = (long) (s - r)) + c + (long) strlen(e);        /* new length */
+                               if ((c = (unsigned long) strlen(ent[b])) - 1 > (e = strchr(s, ';')) - s) {
+                                       l = (d = (unsigned long) (s - r)) + c + (unsigned long) strlen(e);      /* new length */
                                        if (l) {
                                                if (r == m) {
                                                        char *tmp = (char *) malloc(l);
@@ -618,7 +629,7 @@ static char *switch_xml_decode(char *s, char **ent, char t)
 
        if (t == '*') {                         /* normalize spaces for non-cdata attributes */
                for (s = r; *s; s++) {
-                       if ((l = (long) strspn(s, " ")))
+                       if ((l = (unsigned long) strspn(s, " ")))
                                memmove(s, s + l, strlen(s + l) + 1);
                        while (*s && *s != ' ')
                                s++;