Skip to content

fix: sanitize SESSION_ID to prevent path traversal - #138

Closed
kaissb wants to merge 1 commit into
MemPalace:mainfrom
kaissb:fix/session-id-sanitization
Closed

fix: sanitize SESSION_ID to prevent path traversal#138
kaissb wants to merge 1 commit into
MemPalace:mainfrom
kaissb:fix/session-id-sanitization

Conversation

@kaissb

@kaissb kaissb commented Apr 7, 2026

Copy link
Copy Markdown

Strip all characters except alphanumeric, underscore, and hyphen using tr -cd '[:alnum:]_-'.
Fall back to 'unknown' if result is empty.

Fixes #121

What does this PR do?

Sanitizes SESSION_ID in both hook scripts to prevent path traversal and log injection. A
malicious session_id like ../../etc/cron.d/evil could write files outside $STATE_DIR or
inject content into logs.

Affected files:

  • hooks/mempal_save_hook.sh
  • hooks/mempal_precompact_hook.sh

How to test

# Malicious input gets stripped
echo "../../etc/cron.d/evil" | tr -cd '[:alnum:]_-'
# Output: etccrondevil

# Normal input unchanged
echo "abc123-session_01" | tr -cd '[:alnum:]_-'
# Output: abc123-session_01

# Empty result falls back to "unknown"
SANITIZED=$(echo "../../../" | tr -cd '[:alnum:]_-')
[ -z "$SANITIZED" ] && echo "unknown"
# Output: unknown

Checklist

- Tests pass (python -m pytest tests/ -v) — N/A, bash scripts only
- No hardcoded paths
- Linter passes (ruff check .) — N/A, bash scripts only

Strip all characters except alphanumeric, underscore, and hyphen using tr -cd '[:alnum:]_-'. Fall back to 'unknown' if result is empty.

Fixes MemPalace#121
@bensig

bensig commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Duplicate of #141 and #62 which cover the same SESSION_ID sanitization. Closing.

@bensig bensig closed this Apr 7, 2026
@kaissb

kaissb commented Apr 7, 2026

Copy link
Copy Markdown
Author

Duplicate of #141 and #62 which cover the same SESSION_ID sanitization. Closing.

Thanks for the review. Just noting that this PR covered both mempal_save_hook.sh and mempal_precompact_hook.sh, while #62 only addresses the save hook. You may want to ensure the precompact hook gets the same fix. Thx.

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.

Security: SESSION_ID from untrusted JSON used unsanitized in hook file paths

2 participants