https://bugs.openldap.org/show_bug.cgi?id=10489
--- Comment #4 from yutengsun1997@gmail.com --- Got it, here is the origial email content.
Subject: Possible memory issue in liblber: ber_skip_tag() / ber_skip_element() / ber_skip_data() — unchecked ber_ptr dereference
Dear OpenLDAP Security Team,
I hope this message finds you well. I have been doing some security research on BER/DER parsing libraries and came across a potential memory issue in liblber that I would appreciate your assessment on.
## Summary
Three functions in liblber dereference `ber->ber_ptr` to cache the next tag byte without verifying that `ber_ptr < ber_end`. When the current element is the last one in the buffer (or when parsing fails), `ber_ptr` may point at or past `ber_end`, resulting in a 1-byte out-of-bounds read.
I wanted to ask: would you consider this a memory safety issue worth addressing?
## Affected Code
**OpenLDAP 2.6.9** (and likely all prior versions). Three locations:
### 1. `ber_skip_tag()` — libraries/liblber/decode.c:274-275
```c ber->ber_ptr = bv.bv_val; ber->ber_tag = *(unsigned char *) ber->ber_ptr; // unconditional dereference ```
This runs regardless of whether `ber_peek_element()` succeeded or failed. Unlike `ber_skip_element()`, there is no `if (tag != LBER_DEFAULT)` guard, so the dereference occurs even on parse failure.
### 2. `ber_skip_element()` — libraries/liblber/decode.c:230-231
```c ber->ber_ptr = bv.bv_val + bv.bv_len; ber->ber_tag = *(unsigned char *) ber->ber_ptr; // OOB when last element ```
When the parsed element is the last one in the buffer, `ber_ptr` is advanced to exactly `ber_end`, and the read goes 1 byte past the buffer.
### 3. `ber_skip_data()` — libraries/liblber/io.c:62-63
```c ber->ber_ptr += actuallen; ber->ber_tag = *(unsigned char *)ber->ber_ptr; // same pattern ```
## Why ber_init() masks the issue
I noticed that `ber_init()` allocates `bv_len + 1` bytes and NUL-terminates the copy, so the out-of-bounds read hits a sentinel byte within the allocation. This effectively masks the issue in many common code paths.
However, `ber_init2()` uses the caller's buffer in-place without extra allocation, which makes the out-of-bounds read a real access past the buffer boundary.
## Reachability via ber_init2()
From what I can see, `ber_init2()` is called in a number of places that process incoming data, including:
- `servers/slapd/cancel.c` — extended operation request data - `servers/slapd/passwd.c` — password modify request data - `servers/slapd/controls.c` — LDAP control values - `servers/slapd/schema_init.c` — DER-encoded attribute values - `servers/lloadd/bind.c`, `servers/lloadd/client.c` — load balancer data - `libraries/libldap/result.c` — client-side server response parsing - `libraries/libldap/tls_g.c:504` — X.509 certificate DN extraction - Various overlays (syncprov, sssvlv, deref, valsort, etc.)
## Reproduction
A minimal example using `ber_init2()`, compiled with AddressSanitizer:
```c #include <lber.h> #include <stdint.h>
int main(void) { // A complete NULL element: tag=0x05, length=0x00 uint8_t buf[2] = {0x05, 0x00};
BerElement *ber = ber_alloc_t(0); struct berval bv; bv.bv_val = (char *)buf; bv.bv_len = sizeof(buf); ber_init2(ber, &bv, 0);
// ber_skip_tag() advances ber_ptr to buf[2] (past end), // then dereferences it to cache ber_tag. ber_len_t len = 0; ber_skip_tag(ber, &len);
ber_free(ber, 0); return 0; } ```
Compile and run: ```bash clang -g -O1 -fsanitize=address,undefined -I<openldap>/include \ -DLBER_LIBRARY decode.c io.c memory.c options.c sockbuf.c \ bprint.c debug.c assert.c encode.c stdio.c test.c -o test ./test ```
ASan reports: ``` ERROR: AddressSanitizer: stack-buffer-overflow on address ... READ of size 1 at ... #0 ... in ber_skip_tag .../decode.c:275 ```
## Suggested Fix
A bounds check before each `ber_ptr` dereference would address this:
```c if (ber->ber_ptr < ber->ber_end) ber->ber_tag = *(unsigned char *) ber->ber_ptr; else ber->ber_tag = LBER_DEFAULT; ```
Additionally, `ber_skip_tag()` could benefit from the same `if (tag != LBER_DEFAULT)` guard that `ber_skip_element()` already has, to avoid the dereference on parse failure.
## Notes
- I understand that `ber_init()` mitigates this in practice for many code paths. I am reporting this primarily because `ber_init2()` is a public API and is used internally in places that handle incoming data. - I have only tested this on OpenLDAP 2.6.9 but the code pattern appears to have been present for a long time.
Thank you very much for your time and for maintaining OpenLDAP. I would be happy to provide any additional information or testing if that would be helpful.
Best regards.