CVE-2015-5330: ldb_dn: simplify and fix ldb_dn_escape_internal()

Previously we relied on NUL terminated strings and jumped back and
forth between copying escaped bytes and memcpy()ing un-escaped chunks.
This simple version is easier to reason about and works with
unterminated strings. It may also be faster as it avoids reading the
string twice (first with strcspn, then with memcpy).

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Pair-programmed-with: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
index 6b6f90c13ea00914315fac90906eb888352372d7..1b8e51e499059d35d4f57de0163bde1e6d747c04 100644 (file)
--- a/lib/ldb/common/ldb_dn.c
+++ b/lib/ldb/common/ldb_dn.c
@@ -189,33 +189,23 @@ struct ldb_dn *ldb_dn_new_fmt(TALLOC_CTX *mem_ctx,
 /* see RFC2253 section 2.4 */
 static int ldb_dn_escape_internal(char *dst, const char *src, int len)
 {
-       const char *p, *s;
+       char c;
        char *d;
-       size_t l;
-
-       p = s = src;
+       int i;
        d = dst;
 
-       while (p - src < len) {
-               p += strcspn(p, ",=\n\r+<>#;\\\" ");
-
-               if (p - src == len) /* found no escapable chars */
-                       break;
-
-               /* copy the part of the string before the stop */
-               memcpy(d, s, p - s);
-               d += (p - s); /* move to current position */
-               
-               switch (*p) {
+       for (i = 0; i < len; i++){
+               c = src[i];
+               switch (c) {
                case ' ':
-                       if (p == src || (p-src)==(len-1)) {
+                       if (i == 0 || i == len - 1) {
                                /* if at the beginning or end
                                 * of the string then escape */
                                *d++ = '\\';
-                               *d++ = *p++;                                     
+                               *d++ = c;
                        } else {
                                /* otherwise don't escape */
-                               *d++ = *p++;
+                               *d++ = c;
                        }
                        break;
 
@@ -231,30 +221,30 @@ static int ldb_dn_escape_internal(char *dst, const char *src, int len)
                case '?':
                        /* these must be escaped using \c form */
                        *d++ = '\\';
-                       *d++ = *p++;
+                       *d++ = c;
                        break;
 
-               default: {
+               case ';':
+               case '\r':
+               case '\n':
+               case '=':
+               case '\0': {
                        /* any others get \XX form */
                        unsigned char v;
                        const char *hexbytes = "0123456789ABCDEF";
-                       v = *(const unsigned char *)p;
+                       v = (const unsigned char)c;
                        *d++ = '\\';
                        *d++ = hexbytes[v>>4];
                        *d++ = hexbytes[v&0xF];
-                       p++;
                        break;
                }
+               default:
+                       *d++ = c;
                }
-               s = p; /* move forward */
        }
 
-       /* copy the last part (with zero) and return */
-       l = len - (s - src);
-       memcpy(d, s, l + 1);
-
        /* return the length of the resulting string */
-       return (l + (d - dst));
+       return (d - dst);
 }
 
 char *ldb_dn_escape_value(TALLOC_CTX *mem_ctx, struct ldb_val value)