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 85f89c1b06daf70ec77306158863dba9942e6b7a..d39eb8caec1b208f269d20f97512eed875165fd7 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)