CVE-2026-48488 Root-Cause Fix Review for phpMyFAQ Attachment Key Hashing

Vulnerability

CVE-2026-48488 describes phpMyFAQ storing SHA-1 hashes of custom attachment encryption keys in the database, enabling efficient offline cracking if an attacker obtains database read access. The vulnerable implementation maintained a passwordHash property, populated it with sha1((string) $key) for non-default attachment keys, selected password_hash from storage, and inserted that verifier into the attachment metadata table. Because SHA-1 is fast and unsalted, it is unsuitable for protecting low-entropy user-chosen secrets against offline guessing, especially when the hash directly corresponds to a decryption key verifier rather than a one-way authentication secret. This creates a practical path from database disclosure to attachment decryption.

The code-level issue is visible in the referenced commit and aligns with the vulnerability description in the public records at the upstream patch, NVD, and CVE.org.

// Vulnerable behavior from the provided diff summary
if ($default) {
    return;
}

$this->passwordHash = sha1((string) $key);

// ... later persisted as password_hash in the database

Patch

The patch removes the SHA-1-based verifier flow rather than replacing SHA-1 with another hash. In AbstractAttachment.php, the database SELECT no longer fetches password_hash, the attachment object no longer assigns $this->passwordHash from database state, the setKey() API is simplified from setKey(?string $key, bool $default = true) to setKey(?string $key), and the INSERT statement no longer persists a password_hash column value. Corresponding tests were updated to remove assertions around passwordHash and the default/custom key branching behavior.

// Patched behavior from the provided diff summary
public function setKey(?string $key): void

SELECT
    record_id, record_lang, real_hash, virtual_hash,

INSERT INTO
    filename, filesize, encrypted, mime_type)
    (%d, %d, '%s', '%s', '%s', '%s', %d, %d, '%s')

Technically, this changes the security model from “store a fast hash of the custom encryption key” to “do not store a crackable verifier for the custom encryption key in attachment metadata.” That directly removes the database-resident artifact that made offline key recovery feasible.

Review

Pros

• The patch removes the vulnerable primitive entirely instead of attempting a minimal algorithm swap. For this issue class, eliminating stored verifier material is stronger than replacing SHA-1 with another password hash unless a verifier is strictly required.
• The fix is tightly scoped to the affected data flow: object state, SQL SELECT, SQL INSERT, and tests all reflect the removal of password_hash.
• API simplification in setKey(?string $key) reduces branching around “default” versus “custom” key handling, which lowers the chance of reintroducing insecure special-case persistence.
• The patch aligns with the stated impact in NVD: if the database no longer contains SHA-1 key hashes, the disclosed offline cracking avenue is removed.

Cons

• The diff summary does not show any explicit schema migration or cleanup path for legacy password_hash values already stored in deployed databases. Existing rows may remain exposed until separately purged or ignored by all code paths.
• The review material does not show whether attachment decryption, key validation, or UX flows depended on the removed verifier. If any downstream logic still assumes verifier availability, there could be compatibility or operational regressions outside the shown snippets.
• The patch removes the column from application queries and inserts, but the provided evidence does not demonstrate whether all historical read paths, admin tooling, exports, or upgrade routines also stop exposing legacy verifier data.

Verdict

Root-cause.

Based on the provided commit diff, the patch addresses the core weakness by eliminating storage and retrieval of SHA-1-derived attachment key hashes rather than merely substituting a different fast hash or adding superficial checks. The vulnerability exists because database disclosure yields a reusable offline cracking target; removing password_hash from the attachment persistence model directly breaks that attack path. The main residual concern is migration hygiene for already-stored hashes, but that is a deployment and cleanup consideration, not evidence that the code fix itself is only partial. See the upstream commit, NVD, and CVE.org for the referenced source context.