openwrt / packages

Community maintained packages for OpenWrt. Documentation for submitting pull requests is in CONTRIBUTING.md
GNU General Public License v2.0
3.95k stars 3.46k forks source link

Openldap - Memory Leak - ldap_set_options - New context for Secure LDAP scenario (Cert Available) #20530

Open pankghos opened 1 year ago

pankghos commented 1 year ago

Dear fellow developers,

We are facing a leak in ldap_set_option -

While doing - ldap_rc = ldap_set_option(*ldp, LDAP_OPT_X_TLS_NEWCTX, &tls_option);

while in openldap code a memory allocation of - BIO_meth_new( int type, const char name ) { BIO_METHOD method = LDAP_MALLOC( sizeof(BIO_METHOD) ); memset( method, 0, sizeof(BIO_METHOD) );

    method->type = type;
    method->name = name;

    return method;

}

The BIO method which is a global context allocated dynamically in LDAP_MALLOC is not gettibg frred, resulting a 48 b byte increase in valgrind leak analysis -

==2427== 61 (48 direct, 13 indirect) bytes in 1 blocks are definitely lost in loss record 494 of 1,015 ==2427== at 0x4848EE4: malloc (vg_replace_malloc.c:393) ==2427== by 0x4A71BA7: CRYPTO_zalloc (mem.c:230) ==2427== by 0x49B677F: BIO_meth_new (bio_meth.c:38) ==2427== by 0xA1CB7AB: tlso_bio_setup (tls_o.c:1311) ==2427== by 0xA1CB7AB: tlso_init (tls_o.c:234) ==2427== by 0xA1C7577: ldap_int_tls_init_ctx (tls2.c:211) ==2427== by 0xA1C8547: ldap_pvt_tls_set_option (tls2.c:991) ==2427== by 0xA1BD617: ldap_set_option (options.c:860) ==2427== by 0xA0BACCB: _init_bmc_ldap (pam_bmc_ldap.c:1318) ==2427== by 0xA0BDD37: ldap_auth (pam_bmc_ldap.c:1920) ==2427== by 0xA0B3BA3: pam_sm_authenticate (pam_bmc.c:1255) ==2427== by 0x4E776DF: _pam_dispatch_aux (pam_dispatch.c:110) ==2427== by 0x4E776DF: _pam_dispatch (pam_dispatch.c:426) ==2427== by 0x4E77063: pam_authenticate (pam_auth.c:34) ==2427== by 0x4E618FB: redfish_pam_authenticate (redfish_pam_handler.c:250) ==2427== by 0x4E5FF3F: redfish_data_pam_login (redfish_data_handler.c:154) ==2427== by 0x4E60C9F: redfish_do_basic_auth (redfish_data_handler.c:570) ==2427== by 0xF338F: verify_authentication (oem_authentication.c:182) ==2427== by 0x1AE2B: request_process (request.c:1616) ==2427== by 0xF0EEB: fcgi_worker_thread (fcgi.c:553) ==2427== by 0x517736B: start_thread (pthread_create.c:486) ==2427== by 0x52A9B57: ??? (clone.S:73)

We tried to prevent it by calling - ldap_pvt_tls_destroy() after each ldap_unbind_ext_s(*ldp, NULL, NULL)

But in scenarios where simultaneously ldap_initialize() is called from global context freeing by ldap_pvt_tls_destroy() causing core.

If there is any getway with this scenario, please let us know.

Quick responses are highly appreciated.

Regards, Pankaj Ghosh

brada4 commented 1 year ago

Is there any OpenWRT involved?

feckert commented 1 year ago

If this issue is caused by a patch of openwrt packages feed, then we should fix it.Either by the packages maitainer or by anyone else here in openwrt community.

However, if it is an upstream issue then we have several options.

  1. fix it in the openwrt package feed with a new patch and then send it to the upstream project.
  2. update to a new version maybe it is already fixed.
  3. open an issue in the upstream project and hope that they have a fix for this and backport it the package in openwrt
brada4 commented 1 year ago

Calling _MALLOC macro without _FREE later and rewriting pointer storage yields obvious memory leak. Google says pam redfish module is part of OpenBMC, i.e server management processor firmware, not OpenWRT - home router postmarket firmware.

Neustradamus commented 1 year ago

@flyn-org, @val-kulkov, @neheb, @lynxis, @mhei, @sbyx: Have you seen this ticket?

flyn-org commented 1 year ago

This does not look like a bug in OpenWrt.