s4-dsdb guard principalName parse for invalid inputs

We need to ensure that if this parses name.name_string as just one
val, then we don't read uninitialised and possibly unallocated memory.
Found by Adam Thorn <alt36@cam.ac.uk>

While we are checking that, we need to fix the strncasecmp() check to
first check if the string is the expected length, then check for a
match against sAMAccountName-without-doller, as otherwise we will
permit a string such as machinefoo to match a sAMAccountName of
machine.

Andrew Bartlett

Autobuild-User: Andrew Bartlett <abartlet@samba.org>
Autobuild-Date: Fri Jul  1 03:55:00 CEST 2011 on sn-devel-104

diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index 98bf43d21ea90c29fffc42e464b34cdbee1e9456..49152d418a2cae543575c484230a20e2b2353556 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -477,6 +477,10 @@ static int acl_validate_spn_value(TALLOC_CTX *mem_ctx,
                return LDB_ERR_CONSTRAINT_VIOLATION;
        }
 
+       if (principal->name.name_string.len < 2) {
+               goto fail;
+       }
+
        instanceName = principal->name.name_string.val[1];
        serviceType = principal->name.name_string.val[0];
        realm = krb5_principal_get_realm(krb_ctx, principal);
@@ -509,7 +513,8 @@ static int acl_validate_spn_value(TALLOC_CTX *mem_ctx,
        }
        /* instanceName can be samAccountName without $ or dnsHostName
         * or "ntds_guid._msdcs.forest_domain for DC objects */
-       if (strncasecmp(instanceName, samAccountName, strlen(samAccountName) - 1) == 0) {
+       if (strlen(instanceName) == (strlen(samAccountName) - 1)
+           && strncasecmp(instanceName, samAccountName, strlen(samAccountName) - 1) == 0) {
                goto success;
        } else if (strcasecmp(instanceName, dnsHostName) == 0) {
                goto success;