Skip to content

security: block NQ Vault path traversal on decrypted file writes#669

Merged
stark4n6 merged 2 commits intoabrignoni:mainfrom
mobasi-team:security/nq-vault-path-traversal-fix
Mar 4, 2026
Merged

security: block NQ Vault path traversal on decrypted file writes#669
stark4n6 merged 2 commits intoabrignoni:mainfrom
mobasi-team:security/nq-vault-path-traversal-fix

Conversation

@mobasi-team
Copy link
Copy Markdown
Contributor

Commit

  • 0cafd8f - security: block NQ Vault path traversal on decrypted file writes

Security issue

NQ_Vault.py used attacker-controlled file_name_from from DB directly in:

  • open(join(report_folder, decrypted_file_name), 'wb')

This allowed traversal values like ../../../outside_written.bin to write outside the report output tree.

Vulnerability verification

Confirmed as real in a controlled local PoC on pre-fix code:

  • Input old_filename = '../../../outside_written.bin'
  • Output showed:
    • outside_exists: True
    • is_outside_report: True

Fix implemented

  • Added filename/path hardening helpers in NQ_Vault.py:
    • _sanitize_output_filename(...)
    • _build_safe_output_path(...)
  • Enforces:
    • basename extraction (drop path components)
    • invalid character stripping
    • dot/empty fallback handling
    • resolved-path confinement to report_folder
    • collision-safe output naming
  • Replaced vulnerable write with safe resolved output path.

Regression tests added

  • New test file:
    • admin/test/scripts/test_nq_vault_path_security.py
  • Covers:
    • traversal sanitization
    • output-path confinement to report folder
    • end-to-end file_decryption traversal-block behavior

QA run

  • python3 admin/test/scripts/test_nq_vault_path_security.py
  • python3 -m py_compile scripts/artifacts/NQ_Vault.py admin/test/scripts/test_nq_vault_path_security.py

@abrignoni abrignoni requested review from abrignoni and stark4n6 March 3, 2026 03:06
Copy link
Copy Markdown
Owner

@abrignoni abrignoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. Sanitizes names and path.

@stark4n6
Copy link
Copy Markdown
Collaborator

stark4n6 commented Mar 4, 2026

@mobasi-team @abrignoni looks good to me as well, I'm wondering if the sanitization should be utilized as a function for other parsers too, something we can look at

@stark4n6 stark4n6 merged commit 9b2a9b8 into abrignoni:main Mar 4, 2026
1 check passed
@mobasi-team
Copy link
Copy Markdown
Contributor Author

@stark4n6 @abrignoni

We found these as being places where there's a risk of file path traversal, but it's much harder for an attacker to take advantage of these.

Harder to exploit, but present:

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.

3 participants