Skip to content

Commit 2b4e78b

Browse files
Stop leaking share material to /tmp and to other users
Two findings from a full read of src/, both reproduced before fixing. Extracted share archives were left on disk whenever anything went wrong. The rmtree sat at the end of the loop body, so it was skipped both by the `raise` on an invalid share and by the `continue` on an unreadable one. A single failed decrypt left /tmp/fractum_share_* holding share_N.txt and, when the archive had been built with --bundle-encrypted, the ciphertext beside it. Accumulate enough of those and the leftovers alone are enough to reconstruct. Cleanup moves into a finally. Nothing set permissions on any output, so the process umask decided them: the share archives, the .enc file and the reconstructed plaintext all landed as -rw-r--r--, readable by every user on the machine. They are now created 0600. Verified: a share with corrupt JSON now leaves zero temp directories, the three outputs are -rw-------, the encrypt/decrypt round trip through the official image with --network=none returns an identical SHA-256, and the suite is still 107 tests, 8/8 files.
1 parent 2497888 commit 2b4e78b

3 files changed

Lines changed: 20 additions & 4 deletions

File tree

src/cli/commands.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -580,10 +580,18 @@ def decrypt(
580580
if verbose:
581581
click.echo(f"Error processing {share_file}: {str(e)}")
582582
continue
583-
584-
# Clean up temporary directory if it was an archive
585-
if is_zip and temp_dir:
586-
shutil.rmtree(temp_dir, ignore_errors=True)
583+
finally:
584+
# Quoi qu'il arrive, l'archive extraite ne reste pas sur le
585+
# disque : elle contient la valeur de la part, et le fichier
586+
# chiffre lui-meme si --bundle-encrypted a ete utilise.
587+
if is_zip and temp_dir:
588+
shutil.rmtree(temp_dir, ignore_errors=True)
589+
590+
# Nettoyage deplace dans le finally ci-dessous : place ici, il etait
591+
# saute par le `raise` d une part invalide comme par le `continue`
592+
# d une part illisible, et le dossier restait sur le disque avec la
593+
# valeur de la part extraite, et le fichier chiffre si l archive
594+
# avait ete creee avec --bundle-encrypted.
587595

588596
if verbose:
589597
if shares_by_set_id:

src/crypto/encryption.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import json
2+
import os
23
import time
34
from typing import Any, Dict, Optional, Union
45

@@ -85,6 +86,7 @@ def encrypt_file(
8586
ciphertext, tag = cipher.encrypt_and_digest(data)
8687

8788
with open(output_path, "wb") as f:
89+
os.chmod(output_path, 0o600)
8890
f.write(len(metadata_bytes).to_bytes(4, "big"))
8991
f.write(metadata_bytes)
9092
f.write(cipher.nonce)
@@ -140,6 +142,9 @@ def decrypt_file(self, input_path: str, output_path: str) -> None:
140142
raise ValueError("Decrypted data is empty")
141143

142144
with open(output_path, "wb") as f:
145+
# Le fichier reconstitue est le secret lui-meme : il ne doit pas
146+
# heriter d un umask permissif.
147+
os.chmod(output_path, 0o600)
143148
f.write(data)
144149

145150
except (ValueError, IOError) as e:

src/shares/archiver.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,9 @@ def create_share_archive(
116116
counter += 1
117117

118118
with zipfile.ZipFile(archive_path, "w", zipfile.ZIP_DEFLATED) as zipf:
119+
# L archive porte une part du secret : droits restreints des sa
120+
# creation, avant meme d y ecrire quoi que ce soit.
121+
os.chmod(archive_path, 0o600)
119122
for root, _, files in os.walk(temp_dir):
120123
for file in files:
121124
file_path = Path(root) / file

0 commit comments

Comments
 (0)