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

          Issue ID: 10449
           Summary: memory leak in parseReadAttrs
           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: ---

# ASAN Leak Report: Pre-read / Post-read Control Attributes

## Component
*   **File:** `servers/slapd/controls.c`
*   **Function:** `parseReadAttrs`

## Issue Description
**LeakSanitizer Error:** Memory leak detected in `AttributeName` array.

```plaintext
==202371==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0x555555963614 in malloc
/src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:67:3
    #1 0x555555f578d4 in ber_memalloc_x
/src/openldap/libraries/liblber/memory.c:228:9
    #2 0x555555f4c5e2 in ber_get_stringbvl
/src/openldap/libraries/liblber/decode.c:421:14
    #3 0x555555f4ac2f in ber_scanf
/src/openldap/libraries/liblber/decode.c:815:9
    #4 0x555555b150e4 in parseReadAttrs
/src/openldap/servers/slapd/controls.c:1514:7
    #5 0x555555b0d13c in slap_parse_ctrl
/src/openldap/servers/slapd/controls.c:738:10
    #6 0x555555b0e101 in get_ctrls2
/src/openldap/servers/slapd/controls.c:929:16
    #7 0x555555a35c0e in do_modrdn /src/openldap/servers/slapd/modrdn.c:129:6
    #8 0x5555559e055d in connection_operation
/src/openldap/servers/slapd/connection.c:1137:7
    #9 0x5555559df327 in connection_read_thread
/src/openldap/servers/slapd/connection.c:1289:14
    #10 0x5555559a769a in LLVMFuzzerTestOneInput /src/fuzz_slapd.c:88:5
```
The function `parseReadAttrs` uses `ber_scanf` with the `M` format specifier to
allocate an array of `AttributeName` structures.
1.  **Memory Leak:** This array was not freed if the function encountered an
error (e.g., critical control verification failure) after the allocation.
2.  **Invalid Free:** Attempting to free `bv_val` strings inside the array
caused an "attempting free on address which was not malloc()-ed" error, because
these pointers refer to the internal `BerElement` buffer.
3.  **Segmentation Fault:** Accessing the array to write a terminator without
checking for `NULL` caused a crash on empty attribute lists.

## Fix
*   **Leak Fix:** Added comprehensive cleanup logic at the `done` label to free
the `AttributeName` array if an error occurs.
*   **Logic Fix:** Refactored the loop to filter attributes without attempting
to free the internal string pointers, avoiding the `Invalid Free` error.

## Diff
```diff
diff --git a/servers/slapd/controls.c b/servers/slapd/controls.c
index 3d18345..3c90884 100644
--- a/servers/slapd/controls.c
+++ b/servers/slapd/controls.c
@@ -1476,7 +1476,7 @@ parseReadAttrs(
        LDAPControl *ctrl,
        int post )
 {
-       ber_len_t       siz, off, i;
+       ber_len_t       siz, off, i, good_i = 0;
        BerElement      *ber;
        AttributeName   *an = NULL;

@@ -1520,14 +1520,16 @@ parseReadAttrs(
        for ( i = 0; i < siz; i++ ) {
                const char      *dummy = NULL;
                int             rc;
+               struct berval name = an[i].an_name;

                an[i].an_desc = NULL;
                an[i].an_oc = NULL;
                an[i].an_flags = 0;
-               rc = slap_bv2ad( &an[i].an_name, &an[i].an_desc, &dummy );
+               rc = slap_bv2ad( &name, &an[i].an_desc, &dummy );
                if ( rc == LDAP_SUCCESS ) {
-                       an[i].an_name = an[i].an_desc->ad_cname;
-
+                       an[good_i] = an[i];
+                       an[good_i].an_name = an[good_i].an_desc->ad_cname;
+                       good_i++;
                } else {
                        int                     j;
                        static struct berval    special_attrs[] = {
@@ -1537,23 +1539,29 @@ parseReadAttrs(
                                BER_BVNULL
                        };

-                       /* deal with special attribute types */
                        for ( j = 0; !BER_BVISNULL( &special_attrs[ j ] ); j++
) {
-                               if ( bvmatch( &an[i].an_name, &special_attrs[ j
] ) ) {
-                                       an[i].an_name = special_attrs[ j ];
+                               if ( bvmatch( &name, &special_attrs[ j ] ) ) {
+                                       an[good_i] = an[i];
+                                       an[good_i].an_name = special_attrs[ j
];
+                                       good_i++;
                                        break;
                                }
                        }

-                       if ( BER_BVISNULL( &special_attrs[ j ] ) &&
ctrl->ldctl_iscritical ) {
-                               rs->sr_err = rc;
-                               rs->sr_text = dummy ? dummy
-                                       : READMSG( post, "unknown
attributeType" );
-                               goto done;
+                       if ( BER_BVISNULL( &special_attrs[ j ] ) ) {
+                               if ( ctrl->ldctl_iscritical ) {
+                                       rs->sr_err = rc;
+                                       rs->sr_text = dummy ? dummy
+                                               : READMSG( post, "unknown
attributeType" );
+                                       goto done;
+                               }
                        }
                }
        }

+       if ( an )
+               BER_BVZERO( &an[good_i].an_name );
+
        if ( post ) {
                op->o_postread_attrs = an;
                op->o_postread = ctrl->ldctl_iscritical
@@ -1568,6 +1576,9 @@ parseReadAttrs(

 done:
        (void) ber_free( ber, 1 );
+       if ( rs->sr_err != LDAP_SUCCESS && an ) {
+               ber_memfree( an );
+       }
        return rs->sr_err;
 }
```
## 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