DEV Community

Cover image for Auditing My Own Encryption Tool: 6 Bugs That Looked Fine Until They Weren't
Akhouri Anmol Kumar
Akhouri Anmol Kumar

Posted on

Auditing My Own Encryption Tool: 6 Bugs That Looked Fine Until They Weren't

#productivity #python #coding #testing

Description: "An infinite loop in chunked AEAD, a 2FA lockout, a shredder that wrote one byte, and more. A technical walkthrough of bugs found in ATLOCK v5, a Python file-locking tool."

I build ATLOCK, a Windows file and folder locking tool in Python. It uses
Argon2 key derivation, chunked AES-GCM containers, a password vault with TOTP
2FA, and a "panic password" that opens a decoy vault.

Before shipping the next version, I audited the whole file (about 8,500
lines). Most of what I expected to find, such as OCR-style corruption, bad
indentation and trailing spaces in strings, turned out not to be present. The
interesting bugs were logic bugs in security-sensitive paths. Here are the
real ones, with the reasoning and the fixes.

📦 Availability

ATLOCK v5 is not released yet. It ships when Akhouri Systems crosses
1,000 downloads, and we're currently at 959.

In the meantime you can try ATLOCK v4 here:
https://fn6qtj.s.gy/ATLOCK-v4

Everything described below is part of the v5 work. v5 is the upgrade
path once the milestone is hit.

1. The infinite loop in chunked encryption

ATLOCK writes containers as a stream of AES-GCM chunks. The header declares
the plaintext length up front, and each chunk is authenticated with
associated data (AAD) that binds three things:

  • the header hash
  • the chunk index
  • a flag marking whether it is the final chunk

Simplified, the encrypt loop looked like this:

while True:
    chunk = fh_in.read(chunk_size)
    is_final = (written + len(chunk) >= plain_len)
    nonce = nonce_base + struct.pack(">I", idx)
    aad = header_hash + struct.pack(">I", idx) + (b"\x01" if is_final else b"\x00")
    ct = aes.encrypt(nonce, chunk, aad)
    # ... write, written += len(chunk), idx += 1
    if is_final:
        break
Enter fullscreen mode Exit fullscreen mode

is_final is computed from written, which only grows when read()
returns data. If the source file shrinks while it is being read
(another process truncates it, a sync client rewrites it, a user saves),
read() returns b"". written stops increasing and is_final never
becomes true. The loop spins forever, writing empty encrypted chunks, and the
app freezes.

The tempting fix is wrong

The obvious patch is to force termination:

if not chunk:
    is_final = True
Enter fullscreen mode Exit fullscreen mode

This ends the loop, but it produces a silently corrupt vault. The header
already promises plain_len bytes. The reader derives "is this the last
chunk?" from that length, so it will expect the final-flag AAD at a different
point than the one you wrote. Decryption fails authentication, and you find
out only when the user needs the data. A freeze is better than that, because
at least it is visible.

The fix

Fail loudly, and also stop reading past the declared length so a file that
grows can't be silently truncated:

chunk = fh_in.read(min(chunk_size, max(plain_len - written, 0)) if plain_len else 0)
if not chunk and written < plain_len:
    raise AtlockDamaged(
        "Source file changed size during encryption (it became shorter). "
        "Nothing was replaced; please retry."
    )
is_final = (written + len(chunk) >= plain_len)
Enter fullscreen mode Exit fullscreen mode

I tested four cases: a shrinking source (raises, does not hang), a growing
source (round-trips to exactly the declared length), an empty file, and
normal files.

Lesson: when the format commits to a length in the header, never let the
loop terminate on anything but that length.

2. A 2FA lockout for legacy vaults

Quick Lock re-authenticates the user, then runs the second factor. For
legacy (pre-v3) vaults there is no v3 master section, so the derived
master key is None. The call site did this:

atl_second_factor(db, master or b"", "Quick Lock", ...)
Enter fullscreen mode Exit fullscreen mode

The TOTP seed is stored wrapped under the master key. Unwrapping it with an
empty key always fails authentication. Anyone on a legacy vault with a TOTP
policy would fail Quick Lock every time, with no way back in.

The fix is to derive the real key with the legacy KDF before discarding the
password:

if legacy_ok and master is None:
    with contextlib.suppress(Exception):
        master = atlock_legacy_key(pwd, base64.b64decode(db["master_salt"]))
del pwd
Enter fullscreen mode Exit fullscreen mode

One caveat I verified before claiming too much: TOTP can only be configured
on migrated vaults, so a pure legacy vault has no seed to unwrap. In that
case the existing behaviour (denied, "not set up") is still the correct
fail-closed result. The change makes the code correct instead of relying on
that accident.

Lesson: x or b"" as a default for key material is a smell. A dummy key
doesn't fail safe. It fails permanently.

3. The shredder that wrote one byte

This one is my favourite because two bugs combined.

Bug A: the return value was ignored. After re-encrypting a guarded file,
the code securely deleted the plaintext working copy:

atlock_shred(temp)   # returns False if it couldn't delete
Enter fullscreen mode Exit fullscreen mode

On Windows, if the user leaves the file open in Word or Notepad, the delete
fails with WinError 32 (sharing violation). atlock_shred returned False
correctly, but the caller dropped its "pending re-encrypt" record anyway. The
result was a plaintext file sitting on disk with nothing tracking it.

The fix is to treat a failed shred as a failed operation, so the retry timer
and the on-startup recovery keep the record:

if not atlock_shred(temp) and os.path.exists(temp):
    raise PermissionError("working copy is still open in another program")
Enter fullscreen mode Exit fullscreen mode

I also added a "Please close the file" notification on the first failure,
instead of leaving the user to find out from a log line. Retries run every 15
seconds for up to 10 minutes.

Bug B: the zero-fill pass wrote 1 byte.

f.write(b"\x00" if p == 0 else secrets.token_bytes(n)); left -= n
Enter fullscreen mode Exit fullscreen mode

Pass 0 wrote a single byte per block-sized step, while left was
decremented by n. The zero pass barely touched the file. The random pass
was correct. The fix:

f.write(b"\x00" * n if p == 0 else secrets.token_bytes(n)); left -= n
Enter fullscreen mode Exit fullscreen mode

An honest caveat: on SSDs with wear levelling, or on copy-on-write and
journaling filesystems, overwriting in place is not a guarantee of
irrecoverability anyway. Shredding is best-effort hygiene, not a
forensic-grade guarantee, and the docs should say so.

4. The old panic password survived key rotation

ATLOCK has a panic password that opens a decoy vault. Vaults migrated from the
old format carry that panic slot into the new one. rotate_master replaced
the real master key but left the panic slot untouched, so after rotating
because "my password leaked", the old panic password still opened the
decoy forever.

I can't re-derive the panic hash without the panic password, so the right
move is to retire the slot. I replaced it with an inert slot of the same
shape (so the file doesn't reveal that anything changed) and a fresh random
decoy blob, and told the user to set a new panic password:

if psec0.get("legacy"):
    self._db["panic"] = {
        "salt": base64.b64encode(secrets.token_bytes(16)).decode(),
        "kdf": sec["kdf"],
        "verifier": secrets.token_hex(32),
        "real": False,
    }
    self._db["decoy_entries_enc"] = "gcm3:" + base64.b64encode(secrets.token_bytes(64)).decode()
Enter fullscreen mode Exit fullscreen mode

I also made rotation reject a new master password that equals an active panic
password, which would otherwise make both open the same vault.

A user-chosen panic password on a v3 vault is independent of the master key,
so I left those alone. The bug was specifically the carried-over legacy slot.

5. Hammering the HIBP API

The vault's health audit checks every password against Have I Been Pwned
using the k-anonymity range API. It did so in a tight loop. With 50 or more
entries, you hit rate limits and the later checks fail quietly.

Fixes:

  • time.sleep(0.5) between real requests
  • a per-audit cache, so identical passwords are looked up once
  • stop after the first offline or rate-limited result, instead of waiting for a timeout on every remaining entry 
if pw not in hibp_cache:
    if hibp_calls:
        time.sleep(0.5)
    hibp_calls += 1
    hibp_cache[pw] = atlock_hibp_check(pw)
    if hibp_cache[pw] is None:
        hibp_offline = True
Enter fullscreen mode Exit fullscreen mode

The audit already ran on a worker thread, so the delay doesn't freeze the UI.
A test with 5 unique passwords plus one duplicate made 5 calls and 4 sleeps,
as expected.

6. Compiling with Nuitka: sys.frozen isn't set

This isn't a classic bug, but it would have bitten me in production.
PyInstaller sets sys.frozen. Nuitka does not. My code decided how to
re-launch itself (background agent, Explorer context menu, factory reset)
with:

if getattr(sys, "frozen", False):
    return [sys.executable, "--background-agent"]
Enter fullscreen mode Exit fullscreen mode

In a Nuitka build that check is false, so the code takes the "running as a
.py" path and builds a broken command line. Nuitka injects a module-level
__compiled__ instead:

IS_COMPILED = bool(getattr(sys, "frozen", False)) or ("__compiled__" in globals())

def atl_self_exe() -> str:
    if IS_COMPILED:
        p = os.path.abspath(sys.argv[0]) if sys.argv and sys.argv[0] else sys.executable
        for c in (p, p + ".exe"):
            if os.path.isfile(c):
                return c
    return sys.executable
Enter fullscreen mode Exit fullscreen mode

I also skipped the importlib.util.find_spec dependency pre-check in
compiled mode, since everything is already bundled, and pointed the
release-signature hash at the .exe instead of a .py that no longer exists.

I recommend --standalone over --onefile for this kind of tool. Onefile
unpacks to a temp directory on every launch, which makes self-integrity
checks and registry or scheduled-task paths less predictable.

What I didn't find

An early checklist claimed the file was full of OCR damage: broken dunder
methods, isinstan ce, 1 < < 20, trailing spaces inside every string, and
mixed indentation. I verified each item mechanically and none of it was
there. I used tokenize for the string literals and indentation steps, and
pyflakes plus py_compile for names and syntax.

The lesson is to verify a claimed bug list against the actual file before
"fixing" it. The real bugs were subtler and more dangerous than formatting
problems.

Takeaways

  1. Don't terminate loops on convenience. If a format commits to a length, mismatches should be errors, not quiet terminations.
  2. Placeholder key material (or b"") fails closed forever. Derive the real key, or deny cleanly with a reason.
  3. Always check the result of security-relevant cleanup, such as deletes and shreds. A swallowed False leaves plaintext behind.
  4. Test the helper's internals, not only its API. b"\x00" vs b"\x00" * n passes every "it returned True" test.
  5. Key rotation must cover every credential derived from the old state, including secondary ones like panic and decoy slots.
  6. Know your compiler's markers. Nuitka and PyInstaller differ here.

What I couldn't test on Linux: the Windows-specific parts (the Quick Lock
dialog, real WinError 32 file-lock retries, and the compiled .exe). I
tested those by reading and by simulating the code paths, so I'm calling them
unverified on real Windows until I've run them there.

Try ATLOCK

v5 is coming once Akhouri Systems reaches 1,000 downloads (we're at 959,
so very close). Until then, grab ATLOCK v4 and upgrade when v5 lands:

👉 https://fn6qtj.s.gy/ATLOCK-v4

If you find a bug in the tool, I'd like to hear about it, and I'll write it up
the same way.

Top comments (3)

Collapse
 
akhourianmolkumar profile image
Akhouri Anmol Kumar •

Mustafa ERBAY hehehe ☠️
I'm preparing myself and ATLOCK too.

Collapse
 
akhourianmolkumar profile image
Akhouri Anmol Kumar •

Try ATLOCK v4, break it.
(All of you)
until v5 is rolling out.

Collapse
 
akhourianmolkumar profile image
Akhouri Anmol Kumar •

@xulingfeng bro when you will be free in discord? (online)