da51700 · skrepka · git.iko.soy

git.iko.soy/skrepka

Security audit Q1-Q9: zstd window cap, poll event/body caps, cursor clamp, verify_strict, TEST-NET-2, cursor watchdog, cached X25519 keys, base64 photo validation

commitda517000bfabaf87556a68dd1adeabc2b3b517cf authorLancelot <montelot-dev@typeable.io> dateWed 15 Jul 2026, 21:28 (3 months ago) parente57c03c

7 files changed+114−11

compare with master

unifiedsplit

Cargo.lock+1 Cargo.toml+1 core/Cargo.toml+1 core/src/app.rs+55−2 core/src/crypto.rs+47−8 core/src/protocol.rs+8 server.knot+1−1

Cargo.lock+1

@@ -1797,6 +1797,7 @@ name = "skrepka_core"

17971797version = "0.1.0"

17981798dependencies = [

17991799 "anyhow",

1800+ "base64",

18001801 "boltffi",

18011802 "chacha20poly1305",

18021803 "clap",

Cargo.toml+1

@@ -43,6 +43,7 @@ rand_core = { version = "0.6", features = ["getrandom"] }

4343rand_chacha = "0.3"

4444uuid = { version = "1", features = ["v4"] }

4545hex = "0.4"

46+base64 = "0.22"

4647zeroize = { version = "1", features = ["derive"] }

4748url = "2"

4849

core/Cargo.toml+1

@@ -45,6 +45,7 @@ zstd.workspace = true

4545rand_core.workspace = true

4646uuid.workspace = true

4747hex.workspace = true

48+base64.workspace = true

4849zeroize.workspace = true

4950url.workspace = true

5051

core/src/app.rs+55−2

@@ -58,6 +58,24 @@ const MAX_CONTACTS: usize = 500;

5858/// the budget is dropped exactly like an undecryptable blob would be.

5959const MAX_POLL_TOTAL_BYTES: usize = 64 * 1024 * 1024;

6060

61+/// Cap on the number of events we will attempt to decrypt out of a single poll

62+/// page, independent of the byte budget. Minimum-size blobs (173 bytes decoded)

63+/// would otherwise permit ~388K events within the 64 MiB byte budget, each

64+/// costing a full scalar mult + SHA-512 + AEAD pass — enough to freeze the app

65+/// for tens of seconds per poll. An honest relay caps at 50 events

66+/// (`maxPollEvents`); this is a hostile-relay bound. Set above `MAX_ACK_IDS`

67+/// (1024) so a legitimate large page from one sender still fully processes.

68+const MAX_POLL_EVENTS: usize = 2000;

69+

70+/// Upper bound on a poll response body we will buffer and parse, in bytes.

71+///

72+/// Before the per-blob byte/count budgets in `ingest_poll` ever run, the client

73+/// reads the entire response body into a `Vec<u8>` and deserializes it. A

74+/// multi-gigabyte response from a hostile relay would OOM the app before

75+/// `ingest_poll`'s defenses are consulted. This cap (generously above what 50

76+/// max-size blobs + JSON envelope could cost) bounds the allocation.

77+const MAX_POLL_RESPONSE_BYTES: usize = 64 * 1024 * 1024;

78+

6179/// How long a poll may stay in flight before we assume the shell lost it.

6280///

6381/// `polling` gates the whole long-poll loop, and it is only cleared by a

@@ -1281,16 +1299,34 @@ impl App for Skrepka {

12811299 return render();

12821300 }

12831301 let bytes = resp.take_body().unwrap_or_default();

1302+ // Bound the response before parsing: a hostile relay can return

1303+ // a multi-gigabyte body that OOMs the app before `ingest_poll`'s

1304+ // per-blob budgets are even consulted.

1305+ if bytes.len() > MAX_POLL_RESPONSE_BYTES {

1306+ model.error = Some("relay sent an oversized poll response".into());

1307+ return self.backoff_poll(model);

1308+ }

12841309 let parsed: PollResp = serde_json::from_slice(&bytes).unwrap_or_default();

12851310 let cmd = self.ingest_poll(model, parsed);

12861311 // The re-poll hangs off the *cursor write*, not off this event.

12871312 // Polling again is what acks the batch we just took — the server

12881313 // deletes everything up to `cursor` — so it must not happen until

12891314 // the batch is durable on our side. `SavedCursor` re-polls.

1315+ //

1316+ // Arm a watchdog alongside the cursor write: if the shell ever

1317+ // drops the KV effect (suspension, bug), `SavedCursor` never fires,

1318+ // `polling` stays `false`, and the loop is dead until the next

1319+ // foreground `Connect`. The watchdog fires `Poll` if the loop is

1320+ // still idle — same pattern as the poll/send HTTP watchdogs.

12901321 cmd.and(

12911322 KeyValue::set(K_CURSOR, json_bytes(&model.cursor))

12921323 .then_send(Event::SavedCursor),

12931324 )

1325+ .and(

1326+ Time::notify_after(Duration::from_millis(POLL_WATCHDOG_MS))

1327+ .0

1328+ .then_send(|_| Event::Poll),

1329+ )

12941330 .and(render())

12951331 }

12961332

@@ -1929,8 +1965,13 @@ impl Skrepka {

19291965 return Command::done();

19301966 }

19311967 let prev_cursor = model.cursor;

1932− if page.cursor > model.cursor {

1933− model.cursor = page.cursor;

1968+ // Clamp the relay-supplied cursor: honest cursors are wall-clock-seeded

1969+ // sequence numbers, so anything far in the future is a hostile or buggy

1970+ // relay. Adopting `i64::MAX` would ack (delete) all queued mail on the

1971+ // next poll and persist that poison across relaunches.

1972+ let clamped_cursor = page.cursor.min(now_ms().saturating_add(60_000));

1973+ if clamped_cursor > model.cursor {

1974+ model.cursor = clamped_cursor;

19341975 }

19351976 let Some(id) = model.identity() else {

19361977 return Command::done();

@@ -1945,6 +1986,7 @@ impl Skrepka {

19451986 let mut seen_ids: HashMap<String, HashSet<String>> = HashMap::new();

19461987

19471988 let mut budget = MAX_POLL_TOTAL_BYTES;

1989+ let mut events_processed = 0usize;

19481990 for ev in page.events {

19491991 // Reject an oversized blob on its hex length, before it costs us a

19501992 // decode. No conforming sender produces one (the relay caps blobs at

@@ -1952,6 +1994,17 @@ impl Skrepka {

19521994 if ev.encrypted_blob.len() > MAX_BLOB_LEN * 2 {

19531995 continue;

19541996 }

1997+ // Event count cap: the byte budget alone permits ~388K minimum-size

1998+ // events, each costing a full decrypt attempt. A hostile relay can

1999+ // freeze the app for tens of seconds per poll with a page of small

2000+ // junk blobs. Stop processing past this count, with the same cursor

2001+ // restoration as the byte budget exhaustion.

2002+ if events_processed >= MAX_POLL_EVENTS {

2003+ model.cursor = prev_cursor;

2004+ model.error = Some("relay sent too many poll events".into());

2005+ break;

2006+ }

2007+ events_processed += 1;

19552008 match budget.checked_sub(ev.encrypted_blob.len() / 2) {

19562009 Some(left) => budget = left,

19572010 None => {

core/src/crypto.rs+47−8

@@ -18,7 +18,7 @@ use std::io::Read;

1818use chacha20poly1305::aead::Aead;

1919use chacha20poly1305::{Key, KeyInit, XChaCha20Poly1305, XNonce};

2020use curve25519_dalek::edwards::CompressedEdwardsY;

21−use ed25519_dalek::{Signer, SigningKey, Verifier, VerifyingKey};

21+use ed25519_dalek::{Signer, SigningKey, VerifyingKey};

2222use hkdf::Hkdf;

2323use rand_core::{CryptoRng, RngCore};

2424use sha2::{Digest, Sha256, Sha512};

@@ -127,9 +127,17 @@ impl std::error::Error for CryptoError {}

127127///

128128/// `ZeroizeOnDrop` wipes the seed when the value goes away. `Identity` is cloned

129129/// freely (once per encrypt/decrypt), so each of those copies is wiped too.

130+///

131+/// The derived X25519 keys (`x25519_priv`, `x25519_pub`) are computed once at

132+/// construction and cached: `decrypt` is called per-blob in a poll page, and

133+/// recomputing the SHA-512 + Edwards decompress on every call is a measurable

134+/// CPU multiplier under a flood of small blobs. Both are `Zeroizing` so the

135+/// private half is wiped on drop alongside the seed.

130136#[derive(Clone, ZeroizeOnDrop)]

131137pub struct Identity {

132138 pub secret_key: [u8; ED25519_SECRET_LEN],

139+ x25519_priv: [u8; 32],

140+ x25519_pub: [u8; 32],

133141}

134142

135143impl Identity {

@@ -154,7 +162,7 @@ impl Identity {

154162 return Err(CryptoError::BadKeyLength);

155163 }

156164

157− Ok(Identity { secret_key })

165+ Ok(Identity::with_x25519(secret_key))

158166 }

159167

160168 /// Generate a fresh identity from a CSPRNG.

@@ -171,7 +179,7 @@ impl Identity {

171179 let mut secret_key = [0u8; ED25519_SECRET_LEN];

172180 secret_key[..32].copy_from_slice(seed);

173181 secret_key[32..].copy_from_slice(public.as_bytes());

174− Identity { secret_key }

182+ Identity::with_x25519(secret_key)

175183 }

176184

177185 pub fn public_key(&self) -> [u8; ED25519_PUBLIC_LEN] {

@@ -190,6 +198,29 @@ impl Identity {

190198 SigningKey::from_bytes(&seed)

191199 }

192200

201+ /// Compute the cached X25519 keys from the Ed25519 secret key.

202+ fn with_x25519(secret_key: [u8; ED25519_SECRET_LEN]) -> Self {

203+ let x25519_priv = *ed25519_sk_to_x25519(&secret_key);

204+ let x25519_pub = x25519_dalek::x25519(x25519_priv, x25519_dalek::X25519_BASEPOINT_BYTES);

205+ Identity {

206+ secret_key,

207+ x25519_priv,

208+ x25519_pub,

209+ }

210+ }

211+

212+ /// Cached X25519 private key derived from the Ed25519 seed (libsodium

213+ /// `crypto_sign_ed25519_sk_to_curve25519`). Avoids a per-decrypt SHA-512.

214+ fn x25519_private(&self) -> &[u8; 32] {

215+ &self.x25519_priv

216+ }

217+

218+ /// Cached X25519 public key derived from the Ed25519 public key (libsodium

219+ /// `crypto_sign_ed25519_pk_to_curve25519`). Avoids a per-decrypt decompress.

220+ fn x25519_public(&self) -> &[u8; 32] {

221+ &self.x25519_pub

222+ }

223+

193224 /// Sign the auth challenge: `ed25519(sk, "skrepka-auth-v1:" + host + ":" + challenge)`.

194225 /// `server_host` must already be the bare lowercased hostname.

195226 ///

@@ -271,6 +302,14 @@ fn compress(plaintext: &[u8]) -> Vec<u8> {

271302/// *would* overrun is detected rather than silently truncated.

272303fn decompress(data: &[u8]) -> Result<Vec<u8>, CryptoError> {

273304 let mut decoder = zstd::stream::Decoder::new(data).map_err(|_| CryptoError::Decompress)?;

305+ // Cap the decoder's window buffer so a crafted frame declaring a huge

306+ // windowLog (up to 27 = 128 MiB by default) cannot force a matching

307+ // allocation before the output-size cap even applies. 2^21 = 2 MiB is

308+ // comfortably above `MAX_PLAINTEXT_LEN` (1 MiB), so every legitimate frame

309+ // still decodes; frames declaring a larger window are rejected.

310+ decoder

311+ .window_log_max(21)

312+ .map_err(|_| CryptoError::Decompress)?;

274313 let mut out = Vec::new();

275314 (&mut decoder)

276315 .take(MAX_PLAINTEXT_LEN as u64 + 1)

@@ -386,10 +425,10 @@ pub fn decrypt(recipient: &Identity, blob: &[u8]) -> Result<Decrypted, CryptoErr

386425 nonce.copy_from_slice(&blob[33..33 + NONCE_LEN]);

387426 let ciphertext = &blob[33 + NONCE_LEN..];

388427

389− let recip_x_priv = ed25519_sk_to_x25519(&recipient.secret_key);

390− let raw_secret = x25519(&recip_x_priv, &eph_pub)?;

391− let recip_x_pub = ed25519_pk_to_x25519(&recipient.public_key())?;

392− let key = derive_key(&raw_secret, &eph_pub, &recip_x_pub);

428+ let recip_x_priv = recipient.x25519_private();

429+ let raw_secret = x25519(recip_x_priv, &eph_pub)?;

430+ let recip_x_pub = recipient.x25519_public();

431+ let key = derive_key(&raw_secret, &eph_pub, recip_x_pub);

393432

394433 let cipher = XChaCha20Poly1305::new(Key::from_slice(&*key));

395434 let mut inner = Zeroizing::new(

@@ -427,7 +466,7 @@ pub fn decrypt(recipient: &Identity, blob: &[u8]) -> Result<Decrypted, CryptoErr

427466 signed.extend_from_slice(&recipient.public_key());

428467 signed.extend_from_slice(&inner[96..]);

429468 verifying

430− .verify(&signed, &signature)

469+ .verify_strict(&signed, &signature)

431470 .map_err(|_| CryptoError::BadSignature)?;

432471

433472 let plaintext = decompress(compressed)?;

core/src/protocol.rs+8

@@ -4,6 +4,8 @@

44//! millisecond `ts`. Unknown types are ignored (return `None`).

66use serde::{Deserialize, Serialize};

7+use base64::prelude::*;

8+use base64::Engine;

810/// Caps on the attacker-chosen fields of an incoming payload.

911///

@@ -142,6 +144,12 @@ pub fn parse_payload(bytes: &[u8]) -> Option<ParsedPayload> {

142144 if display_name.chars().count() > MAX_DISPLAY_NAME_LEN

143145 || bio.chars().count() > MAX_BIO_LEN

144146 || photo.as_ref().is_some_and(|p| p.len() > MAX_PHOTO_LEN)

147+ || photo.as_ref().is_some_and(|p| {

148+ // Reject a photo that isn't valid base64 — a peer can put

149+ // any string in the field, and the shell attempts to decode

150+ // it as an image on every render.

151+ BASE64_STANDARD.decode(p).is_err()

152+ })

145153 {

146154 return None;

147155 }

server.knot+1−1

@@ -744,7 +744,7 @@ isBadServerName : Text -> Bool

744744isBadServerName = \raw ->

745745 let sn = stripTrailingDot raw in

746746 let host = beforeColon sn in

747− sn == "" || sn == "localhost" || hasPrefix "localhost:" sn || isNumericHost host || isReservedOctetPrefix host || isReservedIPv4 host || hasLeadingZeroOctet host || hasPrefix "0x" sn || any (\c -> not contains c "0123456789abcdefghijklmnopqrstuvwxyz-.:") (chars sn) || countChar ":" sn > 1 || any (\p -> hasPrefix p sn) [".", ":", "192.0.2.", "203.0.113."] || any (\s -> hasSuffix s sn) [".localhost", ".local", ".internal", ".arpa", ".onion"]

747+ sn == "" || sn == "localhost" || hasPrefix "localhost:" sn || isNumericHost host || isReservedOctetPrefix host || isReservedIPv4 host || hasLeadingZeroOctet host || hasPrefix "0x" sn || any (\c -> not contains c "0123456789abcdefghijklmnopqrstuvwxyz-.:") (chars sn) || countChar ":" sn > 1 || any (\p -> hasPrefix p sn) [".", ":", "192.0.2.", "198.51.100.", "203.0.113."] || any (\s -> hasSuffix s sn) [".localhost", ".local", ".internal", ".arpa", ".onion"]

748748

749749-- ============================================

750750-- Auth helpers