Skip to content

Fix crash on --action get by reading paged_search response dicts (Fixes #5) - #6

Merged
p0dalirius merged 1 commit into
mainfrom
bugfix-paged-search-dict-access
Sep 1, 2026
Merged

p0dalirius merged 1 commit into
mainfrom
bugfix-paged-search-dict-access

Conversation

@p0dalirius

Copy link
Copy Markdown
Owner

Linked Issue

Closes #5

Root Cause

LAPSio.get() treated the values yielded by ldap_session.extend.standard.paged_search() as ldap3 Entry objects and accessed responses[0].entry_dn and responses[0]['objectSid'].raw_values[0]. paged_search() never yields Entry objects: paged_search_generator() yields the items of connection.response, which are the raw decoded LDAP response dictionaries with the keys dn, raw_dn, attributes, raw_attributes and type. Entry objects only ever live in connection.entries, which is why LAPSio.set() — which reads self.ldap_session.entries[0] after a plain search() — was never affected. The mismatch raised AttributeError: 'dict' object has no attribute 'entry_dn' on every --action get run that targets a specific computer, and the surrounding except IndexError did not catch it.

Note on scope: this is not an ldap3 2.9 regression. paged_search yields response dicts in 2.8.1 as well (identical yield responses.pop() / connection.response = responses code in ldap3/extend/standard/PagedSearch.py), so pinning ldap3<2.9 would not have avoided the crash. The bug was in pyLAPS, at every ldap3 version.

The same block held a second defect on the same code path: when the computer was not found, dn stayed None, the code printed [!] Target computer does not exist! and then fell through to a second paged_search(None, ...), crashing with TypeError: cannot convert 'NoneType' object to bytes. Fixing only the dict access would have moved the crash rather than removed it, so both are corrected here.

Fix Description

Replace the Entry-style access in LAPSio.get() with a loop over the paged-search responses that reads entry['dn'] and entry['raw_attributes']['objectSid'], skipping searchResRef referral entries (paged_search interleaves them with searchResEntry items, so responses[0] is not guaranteed to be a result). objectSid is read defensively via .get() so a response without the attribute yields sid = None instead of a KeyError. When no searchResEntry is found, the function now prints the same two existing messages and returns an empty list instead of issuing a search with a None base.

Rejected alternative: pinning ldap3<2.9 in requirements.txt, as suggested in the issue. Verified against ldap3 2.8.1 that paged_search returns dicts there too, so the pin would not fix the crash and would only hold the project back on an old dependency.

No user-facing output strings changed.

How Verified

Runtime. Drove LAPSio.get() against an ldap3 MOCK_SYNC in-memory directory (OFFLINE_AD_2012_R2 schema, one CN=DC01,OU=Domain Controllers,DC=domain,DC=local computer carrying sAMAccountName, objectSid and ms-Mcs-AdmPwd) on ldap3 2.9.1 / Python 3.12, exercising the three code paths.

Before the fix:

=== targeted get, computer exists ===
[+] Searching for the target computer: DC01$
EXCEPTION: AttributeError: 'dict' object has no attribute 'entry_dn'
=== targeted get, computer missing ===
[!] Target computer does not exist! (wrong domain?)
EXCEPTION: TypeError: cannot convert 'NoneType' object to bytes
=== get all ===
RESULT: [{'sAMAccountName': [b'DC01$'], ..., 'ms-Mcs-AdmPwd': [b'Sup3rS3cret!']}]

After the fix:

=== targeted get, computer exists ===
[+] Target computer found: CN=DC01,OU=Domain Controllers,DC=domain,DC=local
RESULT: [{'sAMAccountName': [b'DC01$'], ..., 'ms-Mcs-AdmPwd': [b'Sup3rS3cret!']}]
=== targeted get, computer missing ===
[!] Computer not found in LDAP: NOPE$
[!] Target computer does not exist! (wrong domain?)
RESULT: []
=== get all ===
RESULT: [{'sAMAccountName': [b'DC01$'], ..., 'ms-Mcs-AdmPwd': [b'Sup3rS3cret!']}]

The returned dicts keep the shape __main__ expects (r["sAMAccountName"][0].decode(...), r["ms-Mcs-AdmPwd"][0].decode(...)), so the caller's sort and print are unchanged.

Static. paged_search_generator() in ldap3/extend/standard/PagedSearch.py extends responses from connection.response and yields those items directly; connection.response entries are dicts, never Entry objects. Confirmed by printing type(responses[0]) (<class 'dict'>) and sorted(responses[0].keys()) (['attributes', 'dn', 'raw_attributes', 'raw_dn', 'type']) against the mock server.

CI: python3 -m py_compile pyLAPS.py and python3 pyLAPS.py --help both pass.

Test Coverage

None. The repository has no test suite and no test dependency; CI runs python3 pyLAPS.py --help only. Adding one would mean introducing a test harness and an ldap3 mock-directory fixture, which is beyond a one-function bug fix. The mock-directory driver described above was used to verify the fix and is available if a test suite is added; filing that as separate follow-up work.

Scope of Change

  • Files changed: pyLAPS.py (LAPSio.get(), 13 insertions / 6 deletions)
  • Submodule pointer updated: no
  • Behavioral changes outside the bug fix: none. The only behavior change beyond correct attribute access is the early return [] on the not-found path, which removes the second crash on that same path; output strings are identical.

Risk and Rollout

Confined to LAPSio.get(). --action set is untouched. Worst case for a regression is the get path, which crashed unconditionally before this change, so there is nothing working to break. Safe to merge directly.

Notes

Two pre-existing defects were found in the same file and deliberately left out of this PR to keep it single-purpose; both warrant their own issues:

  1. LAPSio.get() (L293, L310, L317) and LAPSio.set() (L265) use the module-level global ldap_session instead of self.ldap_session. It works only because __main__ happens to bind that global; importing pyLAPS as a module and calling LAPSio.get() raises NameError.
  2. LAPSio.set() (L268–L273) reassigns the loop variable results inside its own for entry in results loop, then tests len(results) == 0 on it, so the "Could not query target computer properties" guard never behaves as intended.

#5)

paged_search() yields raw LDAP response dictionaries, not ldap3 Entry
objects, so `.entry_dn` and `['objectSid'].raw_values` raised
AttributeError in LAPSio.get(). Read `dn` and `raw_attributes` from the
response dict instead, skip searchResRef referrals, and return early when
the target computer is not found so the follow-up search is not issued
with a None search base.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant