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: [email protected]
          Reporter: [email protected]
  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;
}
```

-- 
You are receiving this mail because:
You are on the CC list for the issue.

Reply via email to