Skip to content

fix: harden JSON parsing and path handling in PAM extend#1902

Open
jlima8900 wants to merge 9 commits intoKeeper-Security:releasefrom
jlima8900:fix/pam-extend-security-hardening
Open

fix: harden JSON parsing and path handling in PAM extend#1902
jlima8900 wants to merge 9 commits intoKeeper-Security:releasefrom
jlima8900:fix/pam-extend-security-hardening

Conversation

@jlima8900
Copy link
Copy Markdown
Contributor

@jlima8900 jlima8900 commented Mar 28, 2026

Summary

Hardens JSON parsing and path handling in keepercommander/commands/pam_import/extend.py.

Changes

  • NUL byte stripping — sanitize folder paths before placeholder substitution to prevent injection
  • Specific exception handlers — replace bare except on JSON parse with JSONDecodeError, ValueError, KeyError
  • Path canonicalization — resolve import file paths with os.path.realpath to prevent symlink traversal

Tests

  • CI: pytest 3.7 + 3.12 passing

@jlima8900 jlima8900 changed the base branch from master to release March 30, 2026 13:33
@jlima8900 jlima8900 force-pushed the fix/pam-extend-security-hardening branch from da97c23 to a3e9f59 Compare March 30, 2026 13:39
@jlima8900
Copy link
Copy Markdown
Contributor Author

@aaunario — ready for review. NUL byte stripping, specific exception handlers, path canonicalization.

@jlima8900 jlima8900 force-pushed the fix/pam-extend-security-hardening branch from 6d9359a to b6360e1 Compare April 2, 2026 19:12
idimov-keeper and others added 3 commits April 3, 2026 20:59
* Added full terminal reset after ssh session exit (clears scrollback etc.)

* Fixes randomly eaten first characters typed after ssh session exit (stdin race condition)

* Fixed random duplicate typed characters (echo)
Keeper-Security#1908)

- Added `move` to the list of `apply-action` choices, which will move all returned users to a node - specified with `target-node`

- Added `all` to the list of `status` choices, returning invited, active and locked users with `d=0`

- Fixed an issue where the users updated with `apply-action` are not the same as those filtered with the `node` argument.

- Fixed a potential unwanted behavior where the `node` argument returns a recursive node and subnodes search, preventing you from applying actions to a specific node if it has subnodes. By default, using the `node` argument will only return result from the specified node.

- Added a `recursive` argument to replicate the old behavior with `node` filter.
- Split combined except clause into separate JSONDecodeError and
  UnicodeDecodeError handlers with targeted error messages
- Remove redundant PermissionError (subclass of OSError)
- Add empty-path guard after NUL byte stripping
- Add comment explaining realpath for symlink resolution
@jlima8900 jlima8900 force-pushed the fix/pam-extend-security-hardening branch from b6360e1 to 58c9592 Compare April 4, 2026 10:36
@sk-keeper sk-keeper force-pushed the release branch 3 times, most recently from 442d7af to 9ec9b7a Compare April 15, 2026 03:37
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.

6 participants