[
https://issues.apache.org/jira/browse/ZOOKEEPER-5000?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Cyl updated ZOOKEEPER-5000:
---------------------------
Description:
The function {{zoo_sasl_make_password_callbacks}} in
{{zookeeper-client/zookeeper-client-c/src/zk_sasl.c}} allocates memory for SASL
callbacks and their context data (user, realm, secret) but provides no
mechanism to free the context data.
h2. Location
File: {{zookeeper-client/zookeeper-client-c/src/zk_sasl.c}} Function:
{{zoo_sasl_make_password_callbacks}}
{code:java}
sasl_callback_t *zoo_sasl_make_password_callbacks(const char *user,
const char *realm,
zoo_sasl_password_t *password)
{
// ...
secret_ctx = (struct zsasl_secret_ctx *)calloc(1, sizeof(struct
zsasl_secret_ctx));
// ...
rc = rc < 0 ? rc : _zsasl_strdup(&user_ctx, user);
rc = rc < 0 ? rc : _zsasl_strdup(&realm_ctx, realm);
// ...
sasl_callback_t callbacks[] = {
{ SASL_CB_GETREALM, (sasl_callback_fn_t)&_zsasl_getrealm,
(void*)realm_ctx },
{ SASL_CB_USER, (sasl_callback_fn_t)&_zsasl_simple, (void*)user_ctx
},
// ...
{ SASL_CB_PASS, (sasl_callback_fn_t)&_zsasl_getsecret,
(void*)secret_ctx },
// ...
};
// ...
memcpy(xcallbacks, callbacks, sizeof(callbacks));
return xcallbacks;
} {code}
h2. Impact
Users of the C client library who use SASL authentication with
{{zoo_sasl_make_password_callbacks}} (or {{{}zoo_sasl_make_basic_callbacks{}}})
will leak memory (strings and structs) every time they create these callbacks,
as there is no API to free the internal context data pointed to by the
callbacks. The {{free()}} function only frees the array of
{{{}sasl_callback_t{}}}, not the data pointed to by {{{}context{}}}.
Repeatedly creating and discarding callback arrays slowly grows RSS even when
the caller follows the documented cleanup pattern ({{{}free(callbacks);{}}}).
h2. Fix
Introduce a new API function {{zoo_sasl_free_callbacks}} that properly frees
the callbacks and their associated context data.
{{void zoo_sasl_free_callbacks(sasl_callback_t *callbacks) {
// Iterate and free context data for known callback types
// ...
free(callbacks);
}}}
h2. Proof of Concept
see attachments
Observed output:
{code:java}
Running SASL callback leak PoC for 20000 iterations
Iteration 0 RSS: 3.46 MB
...
Iteration 19000 RSS: 6.12 MB
Done. RSS: 6.26 MB {code}
The RSS grows steadily because each call leaks all duplicated strings and
secret-context allocations referenced by the callbacks.
was:
The function {{zoo_sasl_make_password_callbacks}} in
{{zookeeper-client/zookeeper-client-c/src/zk_sasl.c}} allocates memory for SASL
callbacks and their context data (user, realm, secret) but provides no
mechanism to free the context data.
h2. Location
File: {{zookeeper-client/zookeeper-client-c/src/zk_sasl.c}} Function:
{{zoo_sasl_make_password_callbacks}}
{{sasl_callback_t *zoo_sasl_make_password_callbacks(const char *user,
const char *realm,
zoo_sasl_password_t *password)
\{
// ...
secret_ctx = (struct zsasl_secret_ctx *)calloc(1, sizeof(struct
zsasl_secret_ctx));
// ...
rc = rc < 0 ? rc : _zsasl_strdup(&user_ctx, user);
rc = rc < 0 ? rc : _zsasl_strdup(&realm_ctx, realm);
// ...
sasl_callback_t callbacks[] = {
{ SASL_CB_GETREALM, (sasl_callback_fn_t)&_zsasl_getrealm,
(void*)realm_ctx },
\{ SASL_CB_USER, (sasl_callback_fn_t)&_zsasl_simple,
(void*)user_ctx },
// ...
\{ SASL_CB_PASS, (sasl_callback_fn_t)&_zsasl_getsecret,
(void*)secret_ctx },
// ...
};
// ...
memcpy(xcallbacks, callbacks, sizeof(callbacks));
return xcallbacks;
}}}
h2. Impact
Users of the C client library who use SASL authentication with
{{zoo_sasl_make_password_callbacks}} (or {{{}zoo_sasl_make_basic_callbacks{}}})
will leak memory (strings and structs) every time they create these callbacks,
as there is no API to free the internal context data pointed to by the
callbacks. The {{free()}} function only frees the array of
{{{}sasl_callback_t{}}}, not the data pointed to by {{{}context{}}}.
Repeatedly creating and discarding callback arrays slowly grows RSS even when
the caller follows the documented cleanup pattern ({{{}free(callbacks);{}}}).
h2. Fix
Introduce a new API function {{zoo_sasl_free_callbacks}} that properly frees
the callbacks and their associated context data.
{{void zoo_sasl_free_callbacks(sasl_callback_t *callbacks) \{
// Iterate and free context data for known callback types
// ...
free(callbacks);
}}}
h2. Proof of Concept
Path: {{sasl_leak_poc.c}}
Build (requires the already-built ZooKeeper C client artifacts):
{{cd /zookeeper
gcc -std=c11 -I./zookeeper-client/zookeeper-client-c/include \
-I./zookeeper-client/zookeeper-client-c/generated \
-I./zookeeper-client/zookeeper-client-c \
cve-study/DoS/issue-study/cve-finding/sasl-leak/sasl_leak_poc.c \
-L./zookeeper-client/zookeeper-client-c/.libs -lzookeeper_mt \
-pthread -lm -lrt -lsasl2 \
-o sasl_leak_poc}}
Run (note the {{LD_LIBRARY_PATH}} so {{libzookeeper_mt.so}} is found):
{{cd /zookeeper
LD_LIBRARY_PATH=./zookeeper-client/zookeeper-client-c/.libs \
./sasl_leak_poc 20000}}
Observed output:
{{Running SASL callback leak PoC for 20000 iterations
Iteration 0 RSS: 3.46 MB
...
Iteration 19000 RSS: 6.12 MB
Done. RSS: 6.26 MB}}
The RSS grows steadily because each call leaks all duplicated strings and
secret-context allocations referenced by the callbacks.
> Memory Leak in SASL Callback Generation
> ---------------------------------------
>
> Key: ZOOKEEPER-5000
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-5000
> Project: ZooKeeper
> Issue Type: Bug
> Components: c client
> Affects Versions: 3.9.4
> Reporter: Cyl
> Priority: Major
> Attachments: sasl_leak_poc.c
>
>
> The function {{zoo_sasl_make_password_callbacks}} in
> {{zookeeper-client/zookeeper-client-c/src/zk_sasl.c}} allocates memory for
> SASL callbacks and their context data (user, realm, secret) but provides no
> mechanism to free the context data.
> h2. Location
> File: {{zookeeper-client/zookeeper-client-c/src/zk_sasl.c}} Function:
> {{zoo_sasl_make_password_callbacks}}
>
> {code:java}
> sasl_callback_t *zoo_sasl_make_password_callbacks(const char *user,
> const char *realm,
> zoo_sasl_password_t
> *password)
> {
> // ...
> secret_ctx = (struct zsasl_secret_ctx *)calloc(1, sizeof(struct
> zsasl_secret_ctx));
> // ...
> rc = rc < 0 ? rc : _zsasl_strdup(&user_ctx, user);
> rc = rc < 0 ? rc : _zsasl_strdup(&realm_ctx, realm);
> // ...
> sasl_callback_t callbacks[] = {
> { SASL_CB_GETREALM, (sasl_callback_fn_t)&_zsasl_getrealm,
> (void*)realm_ctx },
> { SASL_CB_USER, (sasl_callback_fn_t)&_zsasl_simple,
> (void*)user_ctx },
> // ...
> { SASL_CB_PASS, (sasl_callback_fn_t)&_zsasl_getsecret,
> (void*)secret_ctx },
> // ...
> };
> // ...
> memcpy(xcallbacks, callbacks, sizeof(callbacks));
> return xcallbacks;
> } {code}
> h2. Impact
> Users of the C client library who use SASL authentication with
> {{zoo_sasl_make_password_callbacks}} (or
> {{{}zoo_sasl_make_basic_callbacks{}}}) will leak memory (strings and structs)
> every time they create these callbacks, as there is no API to free the
> internal context data pointed to by the callbacks. The {{free()}} function
> only frees the array of {{{}sasl_callback_t{}}}, not the data pointed to by
> {{{}context{}}}.
> Repeatedly creating and discarding callback arrays slowly grows RSS even when
> the caller follows the documented cleanup pattern ({{{}free(callbacks);{}}}).
> h2. Fix
> Introduce a new API function {{zoo_sasl_free_callbacks}} that properly frees
> the callbacks and their associated context data.
>
> {{void zoo_sasl_free_callbacks(sasl_callback_t *callbacks) {
> // Iterate and free context data for known callback types
> // ...
> free(callbacks);
> }}}
> h2. Proof of Concept
> see attachments
> Observed output:
> {code:java}
> Running SASL callback leak PoC for 20000 iterations
> Iteration 0 RSS: 3.46 MB
> ...
> Iteration 19000 RSS: 6.12 MB
> Done. RSS: 6.26 MB {code}
> The RSS grows steadily because each call leaks all duplicated strings and
> secret-context allocations referenced by the callbacks.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
