Skip to content

Remove attachment salt hex decode fallback #176

@careck

Description

@careck

Security Review Finding — MEDIUM Priority

Source: Krillnotes Security Review v1.0.1 (April 2026)
Location: krillnotes-core/src/core/workspace/attachments.rs:424

Description

In restore_attachment(), hex::decode(&meta.salt) falls back to meta.salt.as_bytes().to_vec() on decode failure. Malformed hex strings produce incorrect key material, which means:

  1. The derived encryption key will be wrong
  2. Decryption will silently fail or produce garbage
  3. The error is masked — the user sees a corrupt file rather than a clear error message

Impact

A corrupted or tampered salt value in attachment metadata would not be caught — instead of failing fast with a clear error, it silently derives the wrong key.

Recommendation

Remove the unwrap_or_else fallback. The salt should always be valid hex — if it isn't, return an explicit error.

// Before (problematic)
let salt = hex::decode(&meta.salt).unwrap_or_else(|_| meta.salt.as_bytes().to_vec());

// After (correct)
let salt = hex::decode(&meta.salt)
    .map_err(|_| KrillnotesError::CryptoError("invalid attachment salt hex".into()))?;

Acceptance Criteria

  • Fallback removed, replaced with explicit error
  • Test added for malformed salt handling

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsecuritySecurity-related issues

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions