Skip to content

heap-buffer-overflow in value_normalize_ext() — the b3b672b guard is not sufficient #7859

Description

@xhon-pelushi

Following #7758 and #7857, this is the third and last ldap_utf8prev() call site with the same defect. This one matters a bit more than the other two because value.c already carries a guard for it that turns out not to be enough.

b3b672b (issue #7400) added a d <= head break to both trailing-blank loops in value_normalize_ext():

nd = ldap_utf8prev(d);
while (nd && nd >= head && utf8isspace_fast(nd)) {
    d = nd;
    *d = '\0';
    if (d <= head) {
        break;
    }
    nd = ldap_utf8prev(d);
}

That stops d from reaching head, but the walk itself still starts at d and crosses head on its own: ldap_utf8prev() reads the byte before its argument and then keeps walking back while it sees continuation bytes. With d at head + 1 and head[0] a stray continuation byte, it goes straight past the start of the value.

A CIS value of "\xA0 " with TRIM_TRAILING_BLANK is enough — 0xA0 is not a space, so the leading strip leaves it, it is copied to the output, and the trailing space sets prevspace:

ERROR: AddressSanitizer: heap-buffer-overflow on address 0x50200000002f
READ of size 1 at 0x50200000002f thread T0
    #0 ldap_utf8prev
    #1 value_normalize_ext trailing-blank loop
0x50200000002f is located 1 bytes before 3-byte region [0x502000000030,0x502000000033)

Reproduced by extracting the CIS path and both trailing-blank blocks verbatim, together with the real UTF8len, ldap_utf8next, ldap_utf8len, ldap_utf8prev and ldap_utf8isspace bodies, since I cannot build the server here. The SHRINK_TRAILING_BLANK block has the same shape and the same exposure.

Severity looks the same as the other two: a read of at most six bytes before the value, feeding a space test, with nothing returned to the client.

Suggested fix

The bounded walk from #7847/#7858. After three sites needing the identical helper, it would be better as one shared bounded prev in utf8.c rather than a third static copy — I am happy to send it either way, or to consolidate all three into a single PR if you would prefer to review it in one place. Just say which you want and I will reshape them.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions