Issue 7105 - ns-slapd crashes when binding with user with SSHA hashed password#7106
Issue 7105 - ns-slapd crashes when binding with user with SSHA hashed password#7106vashirov wants to merge 1 commit into389ds:mainfrom
Conversation
… password Bug Description: A NULL pointer dereference occurs in `entry_get_rdn_mods()` when processing entries where the RDN attribute name does not match any attribute in the entry, for example: ``` dn: userid=test,dc=example,dc=com uid: test ``` When `slapi_entry_attr_find()` fails to find the attribute, it sets `attr` to NULL and returns -1. The code then enters the conditional block and attempts to dereference `attr` by calling `attr_get_deletion_csn(attr)`, causing a crash. This can occur during bind operations with password encoding updates or any operation that calls `entry_get_rdn_mods()`. Fix Description: Add NULL pointer checks before dereferencing `attr` in `entry_get_rdn_mods()`. Fixes: 389ds#7105
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- csn_rdn_add is declared but never assigned before being passed to value_update_csn—please ensure it’s initialized on all code paths.
- Rather than relying on slapi_entry_attr_find to set attr to NULL on failure, explicitly initialize attr to NULL to avoid any leftover state.
- This block has deeply nested conditionals—consider using early-exit guards (continue/return) to flatten the logic and improve readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- csn_rdn_add is declared but never assigned before being passed to value_update_csn—please ensure it’s initialized on all code paths.
- Rather than relying on slapi_entry_attr_find to set attr to NULL on failure, explicitly initialize attr to NULL to avoid any leftover state.
- This block has deeply nested conditionals—consider using early-exit guards (continue/return) to flatten the logic and improve readability.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
tbordaz
left a comment
tbordaz
left a comment
There was a problem hiding this comment.
Humm I am the bad guy regarding this one. This code is broken since I introduced it in #2918 exactly for the purpose of checking RDN attribute values during MOD/MODRDN.
The patch looks good to me. Did you identify a testcase ?
|
Reproducer is described in #7105, I will add a CI test. |
Okay so I think there is another bug. When adding new-users.ldif.txt the server should create RDN value(s) if it does not exist. It is valid that the entries in the LDIF/ADD does not contains RDN value. But my understanding is that (see here ) those values need to be added. attributes: the list of attributes that, along with those from the
RDN, make up the content of the entry being added.
|
|
@tbordaz, I think it works as expected. 389-ds-base/ldap/schema/00core.ldif Lines 618 to 626 in 3bd703c If I add the following ldif: ldapsearch returns: Do you have an example where it doesn't work? |
|
@vashirov , oppsss I missed that it was alias. So I wonder if we could call 'type_norm = slapi_attr_syntax_normalize(type)' before calling slapi_entry_attr_find(e, type_norm, &attr) ? |
|
Oh, and I missed that the second entry has different Would be the following ldif what you expect after normalization and fixing the missing RDN? |
Yes it looks good to me, it should add the RDN value. If it leads to single valued attribute that should have multiple values then it adds a conflict flag. |
| if (attr) { | ||
| attr_value_find_wsi(attr, &bv, &value); | ||
| value_update_csn(value, CSN_TYPE_VALUE_DISTINGUISHED, csn_rdn_add); | ||
| } |
There was a problem hiding this comment.
Would it make sense to try to refetch the RDN value here? In case it might be added by entry_apply_mods_wsi? (IIUC, it's possible)
With something like:
if (slapi_entry_attr_find(entry, type, &attr) == 0 && attr) {
if (attr_value_find_wsi(attr, &bv, &value) == VALUE_PRESENT) {
value_update_csn(value, CSN_TYPE_VALUE_DISTINGUISHED, csn_rdn_add);
}
}3 participants
Bug Description:
A NULL pointer dereference occurs in
entry_get_rdn_mods()when processing entries where the RDN attribute name does not match any attribute in the entry, for example:When
slapi_entry_attr_find()fails to find the attribute, it setsattrto NULL and returns -1. The code then enters the conditional block and attempts to dereferenceattrby callingattr_get_deletion_csn(attr), causing a crash. This can occur during bind operations with password encoding updates or any operation that callsentry_get_rdn_mods().Fix Description:
Add NULL pointer checks before dereferencing
attrinentry_get_rdn_mods().Fixes: #7105
Summary by Sourcery
Bug Fixes: