Issue 7547 - Heap buffer overflow in ldap_utf8prev()

[IliaKash1]

Bug description:

Heap buffer overflow in ldap_utf8prev() can be triggered via str2simple if '=' is not preceded by proper symbols.

Fix description:

Additional checks are added to account for '=' being preceded by nothing or by non-ASCII bytes.

Fixes: #7547

Author: Ilia Kashintsev

Summary by Sourcery

Bug Fixes:

• Reject filter strings where '=' is the first character or appears in the middle of a UTF-8 multibyte sequence to prevent heap buffer overreads in ldap_utf8prev().

[sourcery-ai]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="ldap/servers/slapd/str2filter.c" line_range="319-323" />
<code_context>
+    if ((s = strchr(str, '=')) == NULL || s == str) {
         return (NULL);
     }
+    for (char *p = s; (((unsigned char)p[-1]) & 0xC0) == 0x80; --p) {
+        if (p - 1 == str) {
+            return NULL;
+        }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard the loop against going past the start of the buffer by including the pointer bound in the loop condition instead of relying on the body.

Currently the safety of `p[-1]` depends on `if (p - 1 == str) return NULL;` executing before `--p` makes `p == str`, which is correct but subtle and easy to break with later changes. Encoding the boundary directly in the loop header, for example:

```c
for (char *p = s; p > str && (((unsigned char)p[-1]) & 0xC0) == 0x80; --p) {
    if (p - 1 == str) {
        return NULL;
    }
}
```
keeps the body focused on semantics and makes the safety invariant obvious, reducing the risk of future out-of-bounds access.

```suggestion
    for (char *p = s; p > str && (((unsigned char)p[-1]) & 0xC0) == 0x80; --p) {
        if (p - 1 == str) {
            return NULL;
        }
    }
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[packit-as-a-service]

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

• sudo dnf install -y 'dnf*-command(copr)'
• dnf copr enable packit/389ds-389-ds-base-7548
• And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

[progier389]

I do not think that we need a for loop here:
Using:

  if (p[-1] & 0x80) {
     return NULL;
  } 

is enough to determine that p[-1] is not an ascii char
and that the filter string is is not compliant to filter ABNF defined in RFC 4515

[progier389]

LGTM

[tbordaz]

This ticket is linked with IDM-6709