[Issue 10450] New: memory leak in parseAssert

https://bugs.openldap.org/show_bug.cgi?id=10450

Issue ID: 10450 Summary: memory leak in parseAssert Product: OpenLDAP Version: 2.6.12 Hardware: x86_64 OS: Linux Status: UNCONFIRMED Keywords: needs_review Severity: normal Priority: --- Component: slapd Assignee: bugs@openldap.org Reporter: kangyang126@gmail.com Target Milestone: ---

# Memory Leak Fix: Control Parsing Error Handling

## Summary Fixed a memory leak in LDAP control parsing caused by improper error handling in `parseAssert` and `parseValuesReturnFilter` functions. The issue resulted in double response sending and disrupted the normal resource cleanup flow.

---

## Problem Description

### Memory Leak Details - **Location**: `servers/slapd/controls.c` - **Affected Functions**: `parseAssert()`, `parseValuesReturnFilter()` - **Leak Size**: 24 bytes per occurrence (Filter structure) - **Trigger**: Control parsing errors or incomplete operation cleanup

### Stack Trace ``` Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 malloc #1 ber_memalloc_x /src/openldap/libraries/liblber/memory.c:228 #2 ch_malloc /src/openldap/servers/slapd/ch_malloc.c:54 #3 get_filter0 /src/openldap/servers/slapd/filter.c:312 #4 get_filter_list /src/openldap/servers/slapd/filter.c:350 #5 get_filter0 /src/openldap/servers/slapd/filter.c:244 #6 get_filter_list /src/openldap/servers/slapd/filter.c:350 #7 get_filter0 /src/openldap/servers/slapd/filter.c:231 #8 parseAssert /src/openldap/servers/slapd/controls.c:1435 #9 slap_parse_ctrl /src/openldap/servers/slapd/controls.c:738 #10 get_ctrls2 /src/openldap/servers/slapd/controls.c:929 #11 do_search /src/openldap/servers/slapd/search.c:194 #12 connection_operation /src/openldap/servers/slapd/connection.c:1137 #13 connection_read_thread /src/openldap/servers/slapd/connection.c:1289 ```

---

## Root Cause Analysis

### Architectural Issue Control parser functions were violating the separation of concerns by: 1. **Sending LDAP responses directly** instead of returning error codes 2. **Manually freeing resources** in error paths instead of relying on cleanup infrastructure 3. **Breaking the normal cleanup flow** established by `slap_op_free()` → `slap_free_ctrls()`

### Code Flow Analysis

#### Before Fix - Incorrect Flow: ``` 1. get_ctrls2() calls slap_parse_ctrl() 2. slap_parse_ctrl() calls parseAssert() 3. parseAssert() calls get_filter() which allocates Filter 4. get_filter() returns error 5. parseAssert() sends LDAP response ← FIRST SEND (wrong!) 6. parseAssert() frees op->o_assertion ← Manual cleanup (wrong!) 7. parseAssert() returns error to slap_parse_ctrl() 8. slap_parse_ctrl() returns error to get_ctrls2() 9. get_ctrls2() goes to return_results: 10. get_ctrls2() sends LDAP response ← SECOND SEND (duplicate!) 11. Operation cleanup may or may not happen 12. If cleanup fails → MEMORY LEAK ```

#### After Fix - Correct Flow: ``` 1. get_ctrls2() calls slap_parse_ctrl() 2. slap_parse_ctrl() calls parseAssert() 3. parseAssert() calls get_filter() which allocates Filter 4. get_filter() returns error 5. parseAssert() returns error (no send, no cleanup) 6. slap_parse_ctrl() returns error to get_ctrls2() 7. get_ctrls2() goes to return_results: 8. get_ctrls2() sends LDAP response ← SINGLE SEND (correct!) 9. Eventually slap_op_free() is called 10. slap_op_free() calls slap_free_ctrls() 11. slap_free_ctrls() properly frees all resources 12. No memory leak ```

---

## The Fix

### Files Modified - `servers/slapd/controls.c`

### Changes Made

#### 1. parseAssert() - Lines 1435-1451 **Removed:** ```c if( rs->sr_err != LDAP_SUCCESS ) { if( rs->sr_err == SLAPD_DISCONNECT ) { rs->sr_err = LDAP_PROTOCOL_ERROR; send_ldap_disconnect( op, rs ); rs->sr_err = SLAPD_DISCONNECT; } else { send_ldap_result( op, rs ); } if( op->o_assertion != NULL ) { filter_free_x( op, op->o_assertion, 1 ); op->o_assertion = NULL; } return rs->sr_err; } ```

**Replaced with:** ```c if( rs->sr_err != LDAP_SUCCESS ) { return rs->sr_err; } ```

#### 2. parseValuesReturnFilter() - Lines 1630-1647 **Removed:** ```c if( rs->sr_err != LDAP_SUCCESS ) { if( rs->sr_err == SLAPD_DISCONNECT ) { rs->sr_err = LDAP_PROTOCOL_ERROR; send_ldap_disconnect( op, rs ); rs->sr_err = SLAPD_DISCONNECT; } else { send_ldap_result( op, rs ); } if( op->o_vrFilter != NULL) { vrFilter_free( op, op->o_vrFilter ); op->o_vrFilter = NULL; } } ```

**Replaced with:** ```c if( rs->sr_err != LDAP_SUCCESS ) { return rs->sr_err; } ```

---

## Technical Details

### Cleanup Infrastructure The proper cleanup is handled by:

1. **slap_op_free()** (`servers/slapd/operation.c:80`) - Called when operations are freed from connection queue - Invokes `slap_free_ctrls(op, op->o_ctrls)` at line 102

2. **slap_free_ctrls()** (`servers/slapd/controls.c:608`) - Checks `if(ctrls == op->o_ctrls)` to ensure cleaning operation controls - Frees `op->o_assertion` via `filter_free_x()` (lines 615-617) - Frees `op->o_vrFilter` via `vrFilter_free()` (lines 619-621) - Handles NULL checks properly, safe for partial parsing

## Harness ``` #define _GNU_SOURCE /* For memfd_create */ #include "portable.h" #include <stdio.h> #include <string.h> #include <unistd.h> #include <fcntl.h> #include <sys/mman.h> #include <sys/types.h> #include <signal.h>

#include "slap.h"

/* 1. Fix function declarations to match return types in OpenLDAP source code */ extern void slap_sl_mem_init( void ); extern int slapd_daemon_init( const char *urls ); extern int extops_init( void ); /* Returns int */ extern void lutil_passwd_init( void ); extern int slap_init( int mode, const char *name ); extern int read_config( const char *fname, const char *dname ); extern int connections_init( void ); /* Returns int */ extern int slap_startup( Backend *be ); void slapd_daemon_nothread( void ) {} extern int ldap_pvt_thread_initialize( void ); /* Returns int */ extern void* ldap_pvt_thread_pool_context( void ); extern void* connection_read_thread( void *ctx, void *arg ); extern void connection_closing( Connection *c, const char *why ); extern int connection_resched( Connection *c ); extern Connection* connection_init( int sfd, Listener *l, const char *authid, const char *dns, int flags, slap_ssf_t ssf, struct berval *authid_out ); extern ldap_pvt_thread_pool_t connection_pool;

static Listener *global_listener = NULL; static struct berval authid_bv = {0, ""};

int LLVMFuzzerInitialize(int *argc, char ***argv) { signal(SIGPIPE, SIG_IGN); slap_debug = 0; slap_sl_mem_init();

if ( 0 != slapd_daemon_init("ldapi://%2Ftmp%2Ffuzz.sock") ) return 1;

extops_init(); lutil_passwd_init();

if ( slap_init(SLAP_SERVER_MODE, "slapd-fuzz") ) return 2;

read_config( NULL, NULL );

connections_init(); slap_startup(NULL);

slapd_daemon_nothread(); ldap_pvt_thread_initialize();

global_listener = slapd_get_listeners()[0]; if (!global_listener) return 3;

return 0; }

#include <sys/socket.h>

int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { if (Size < 2 || Size > 65536) return 0;

/* 2. Use socketpair to simulate socket read/write */ int sv[2]; if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) < 0) return 0;

if (write(sv[0], Data, Size) != (ssize_t)Size) { close(sv[0]); close(sv[1]); return 0; } /* Close write end so server receives EOF after reading data */ close(sv[0]);

/* 3. Create connection (server uses sv[1]) */ Connection *c = connection_init(sv[1], global_listener, "fuzz", "IP=127.0.0.1", 0, 0, &authid_bv); if (!c) { close(sv[1]); return 0; }

void* ctx = ldap_pvt_thread_pool_context();

/* Execute parsing logic */ connection_read_thread(ctx, (void*)(long)sv[1]);

/* Wait for thread pool to complete all tasks to prevent background threads from accessing freed connection */ int active = 0, pending = 0; do { ldap_pvt_thread_pool_query(&connection_pool, LDAP_PVT_THREAD_POOL_PARAM_PENDING, &pending); ldap_pvt_thread_pool_query(&connection_pool, LDAP_PVT_THREAD_POOL_PARAM_ACTIVE, &active); if (active == 0 && pending == 0) break; ldap_pvt_thread_yield(); } while (1);

/* 4. Thoroughly clean up manually, avoiding unstable macros * Directly manipulate queue pointers to ensure no 'error: use of undeclared identifier o_next' */ Operation *op; while ((op = LDAP_STAILQ_FIRST(&c->c_ops)) != NULL) { LDAP_STAILQ_REMOVE_HEAD(&c->c_ops, o_next); LDAP_STAILQ_NEXT(op, o_next) = NULL; slap_op_free(op, ctx); } LDAP_STAILQ_INIT(&c->c_ops);

/* Close and reclaim Connection memory */ connection_closing(c, "fuzz complete"); connection_resched(c);

/* ldap_pvt_thread_pool_resume(&connection_pool); */

return 0; } ```