sin/samba-mirror
1
0
Fork 0
mirror of https://github.com/samba-team/samba.git synced 2025-01-18 06:04:06 +03:00
Code

ldb-samba: ldif-handlers: make ldif_comparison_objectSid() accurate

Browse Source 
This function compares blobs that might be SID strings or might be SID
structures. Until now, if they were both (seemingly) strings, they were
compared as strings, otherwise if either was a string it was converted to
a structure blob, then the blobs were compared. This had two big problems:

1. There is variety in the way a SID can be stringified. For example,
   "s-1-02-3" means the same SID as "S-1-2-3", but those wouldn't compare
   equal.

2. SID comparison was crazily non-transitive. Consider the three values
     a = "S-1-2-3-4-5",
     b = "S-1-9-1",
     c = SID("S-1-11-1"), where c is a struct and the others are string.

   then we had,
     a < b, because the 5th character '2' < '9'.
     a > c, because when converted to a structure, the number of sub-auths
            is the first varying byte. a has 3, c has 0.
     b < c, because after the sub-auth count comes the id_auth value
            (big-endian, which doesn't matter in this case).

That made the function unreliable for sorting, AND for simple equality
tests. Also it leaked.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
This commit is contained in:
Douglas Bagnall 2024-04-10 10:54:31 +12:00 committed by Andrew Bartlett
parent 4af670384a
commit 6722e80d1b
1 changed files with 35 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
lib/ldb-samba/ldif_handlers.c
View File

@ -150,36 +150,47 @@ bool ldif_comparision_objectSid_isString(const struct ldb_val *v)
/* /*
compare two objectSids compare two objectSids
If the SIDs seem to be strings, they are converted to binary form.
*/ */
static int ldif_comparison_objectSid(struct ldb_context *ldb, void *mem_ctx, static int ldif_comparison_objectSid(struct ldb_context *ldb, void *mem_ctx,
const struct ldb_val *v1, const struct ldb_val *v2) const struct ldb_val *v1, const struct ldb_val *v2)
{ {
if (ldif_comparision_objectSid_isString(v1) && ldif_comparision_objectSid_isString(v2)) { bool v1_is_string = ldif_comparision_objectSid_isString(v1);
return ldb_comparison_binary(ldb, mem_ctx, v1, v2); bool v2_is_string = ldif_comparision_objectSid_isString(v2);
} else if (ldif_comparision_objectSid_isString(v1) struct ldb_val parsed_1 = {};
&& !ldif_comparision_objectSid_isString(v2)) { struct ldb_val parsed_2 = {};
struct ldb_val v; int ret;
int ret; /*
if (ldif_read_objectSid(ldb, mem_ctx, v1, &v) != 0) { * If the ldb_vals look like SID strings (i.e. start with "S-"
/* Perhaps not a string after all */ * or "s-"), we try to parse them as such. If that fails, we
return ldb_comparison_binary(ldb, mem_ctx, v1, v2); * assume they are binary SIDs, even though that's not really
* possible -- the first two bytes of a struct dom_sid are the
* version (1), and the number of sub-auths (<= 15), neither
* of which are close to 'S' or '-'.
*/
if (v1_is_string) {
int r = ldif_read_objectSid(ldb, mem_ctx, v1, &parsed_1);
if (r == 0) {
v1 = &parsed_1;
} }
ret = ldb_comparison_binary(ldb, mem_ctx, &v, v2);
talloc_free(v.data);
return ret;
} else if (!ldif_comparision_objectSid_isString(v1)
&& ldif_comparision_objectSid_isString(v2)) {
struct ldb_val v;
int ret;
if (ldif_read_objectSid(ldb, mem_ctx, v2, &v) != 0) {
/* Perhaps not a string after all */
return ldb_comparison_binary(ldb, mem_ctx, v1, v2);
}
ret = ldb_comparison_binary(ldb, mem_ctx, v1, &v);
talloc_free(v.data);
return ret;
} }
return ldb_comparison_binary(ldb, mem_ctx, v1, v2); if (v2_is_string) {
int r = ldif_read_objectSid(ldb, mem_ctx, v2, &parsed_2);
if (r == 0) {
v2 = &parsed_2;
}
}
ret = ldb_comparison_binary(ldb, mem_ctx, v1, v2);
if (v1_is_string) {
TALLOC_FREE(parsed_1.data);
}
if (v2_is_string) {
TALLOC_FREE(parsed_2.data);
}
return ret;
} }
/* /*