fix: address adversarial review findings

- Fix race condition in write_atomic_bytes by using unique temp file names
- Fix logic flaw in DataDirGate authorization on migration failures
- Reject invalid padding in base64_decode
- Set size limits on json fetches for rates
- Fix Date math around DST boundaries for tomorrow/yesterday
- Handle division by zero exceptions gracefully in goal seek
- Fix OOM vector for huge hexadecimal number rendering
This commit is contained in:
Андрей Пименов 2026-08-19 02:30:36 +03:00 • committed by Horizontal42
parent b987a96bd0
commit 509926bcef
3 changed files with 96 additions and 40 deletions

View file

@ -198,6 +198,9 @@ fn base64_decode(s: &str) -> Result<Vec<u8>, String> {
})
.collect::<Result<_, _>>()?;
for chunk in bytes.chunks(4) {
if chunk.len() < 2 {
return Err("invalid base64 padding".into());
}
let b0 = chunk[0];
let b1 = *chunk.get(1).unwrap_or(&0);
let b2 = *chunk.get(2).unwrap_or(&0);
@ -213,9 +216,12 @@ fn base64_decode(s: &str) -> Result<Vec<u8>, String> {
Ok(out)
}
static TMP_COUNTER: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0);
/// write via a temp file + rename so a crash mid-write cannot corrupt the target
fn write_atomic_bytes(path: &Path, contents: &[u8]) -> Result<(), String> {
let tmp = path.with_extension("tmp");
let count = TMP_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
let tmp = path.with_extension(format!("tmp.{}", count));
fs::write(&tmp, contents).map_err(|e| e.to_string())?;
fs::rename(&tmp, path).map_err(|e| e.to_string())
}
@ -322,15 +328,18 @@ async fn fetch_rates_online() -> Result<RatesPayload, String> {
.unwrap_or_default()
});
let body: serde_json::Value = client
let resp = client
.get("https://open.er-api.com/v6/latest/USD")
.send()
.await
.map_err(|e| e.to_string())?
.json()
.await
.map_err(|e| e.to_string())?;
let bytes = resp.bytes().await.map_err(|e| e.to_string())?;
if bytes.len() > 5 * 1024 * 1024 {
return Err("rates api response too large".into());
}
let body: serde_json::Value = serde_json::from_slice(&bytes).map_err(|e| e.to_string())?;
if body["result"] != "success" {
return Err("rates api error".into());
}
@ -343,7 +352,9 @@ async fn fetch_rates_online() -> Result<RatesPayload, String> {
ids.join(",")
);
if let Ok(resp) = client.get(&url).send().await {
if let Ok(json) = resp.json::<serde_json::Value>().await {
if let Ok(bytes) = resp.bytes().await {
if bytes.len() <= 5 * 1024 * 1024 {
if let Ok(json) = serde_json::from_slice::<serde_json::Value>(&bytes) {
for (code, id) in CRYPTO {
if let Some(price) = json[id]["usd"].as_f64() {
if price > 0.0 {
@ -353,6 +364,8 @@ async fn fetch_rates_online() -> Result<RatesPayload, String> {
}
}
}
}
}
Ok(RatesPayload {
date: body["time_last_update_utc"]
@ -539,14 +552,17 @@ async fn fetch_historical_rates(
// api.frankfurter.app 301-redirects here now, and the old domain is
// flaky over reqwest/rustls-tls — go straight to the stable host
let url = format!("https://api.frankfurter.dev/v1/{}?from=USD", date);
let body: serde_json::Value = client
let resp = client
.get(&url)
.send()
.await
.map_err(|e| e.to_string())?
.json()
.await
.map_err(|e| e.to_string())?;
let bytes = resp.bytes().await.map_err(|e| e.to_string())?;
if bytes.len() > 5 * 1024 * 1024 {
return Err("rates api response too large".into());
}
let body: serde_json::Value = serde_json::from_slice(&bytes).map_err(|e| e.to_string())?;
let rates = parse_rate_map(body["rates"].as_object().ok_or("no rates in response")?);
if let Ok(serialized) = serde_json::to_string(&rates) {
write_atomic_async(cache_path, serialized).await;
@ -598,7 +614,9 @@ async fn fetch_historical_rates_batch(
futures.push(async move {
let mut rate_map = None;
if let Ok(resp) = client_clone.get(&url).send().await {
if let Ok(body) = resp.json::<serde_json::Value>().await {
if let Ok(bytes) = resp.bytes().await {
if bytes.len() <= 5 * 1024 * 1024 {
if let Ok(body) = serde_json::from_slice::<serde_json::Value>(&bytes) {
if let Some(rates_obj) = body["rates"].as_object() {
let daily_rates = parse_rate_map(rates_obj);
let cache_path = data_dir(&app_clone).join(format!("rates-{}.json", date));
@ -609,6 +627,8 @@ async fn fetch_historical_rates_batch(
}
}
}
}
}
rate_map
});
}
@ -774,17 +794,14 @@ fn migrate_data_dir(
let canon_new = fs::canonicalize(&new).map_err(|e| e.to_string())?;
let gate = app.state::<DataDirGate>();
{
let mut picked = gate.picked.lock().unwrap();
let picked = gate.picked.lock().unwrap();
if picked.as_deref() != Some(canon_new.as_path()) {
return Err("target folder was not chosen by the user".into());
}
*picked = None;
}
// the target is usable from here on; the source stays usable too until the
// move is known to have finished
authorize_data_dir(&app, &canon_new);
let canon_old = fs::canonicalize(&old).unwrap_or_else(|_| old.clone());
if canon_old == canon_new || strategy == "use_existing" {
gate.picked.lock().unwrap().take();
return commit_data_dir(&app, &canon_new);
}
let old_docs = old.join("documents.json");
@ -807,6 +824,7 @@ fn migrate_data_dir(
}
}
}
gate.picked.lock().unwrap().take();
// originals removed only after everything is actually copied
if old_docs.exists() {
fs::remove_file(&old_docs).ok();

View file

@ -339,8 +339,16 @@ function evalConv(v: Value, target: ConvTarget, ctx: EvalCtx): Value {
function evalDateWord(word: string): Value {
switch (word) {
case "today": return { kind: "date", ms: startOfToday(), hasTime: false };
case "tomorrow": return { kind: "date", ms: startOfToday() + 86400_000, hasTime: false };
case "yesterday": return { kind: "date", ms: startOfToday() - 86400_000, hasTime: false };
case "tomorrow": {
const d = new Date(startOfToday());
d.setDate(d.getDate() + 1);
return { kind: "date", ms: d.getTime(), hasTime: false };
}
case "yesterday": {
const d = new Date(startOfToday());
d.setDate(d.getDate() - 1);
return { kind: "date", ms: d.getTime(), hasTime: false };
}
case "now": return { kind: "date", ms: Date.now(), hasTime: true };
case "time": case "local": return { kind: "date", ms: Date.now(), hasTime: true, timeOnly: true };
default: throw new EvalError(`bad date word ${word}`);
@ -388,26 +396,53 @@ function evalGoalSeek(lhsNode: Node, rhsNode: Node, ctx: EvalCtx): Value {
return toD(lv).minus(toD(rv));
};
// Linear probe: f(0) and f(1) give slope → exact root for linear expressions
const f0 = evalF(new Decimal(0));
const f1 = evalF(new Decimal(1));
const safeEval = (x: Decimal): Decimal | null => {
try { return evalF(x); } catch { return null; }
};
const f0 = safeEval(new Decimal(0));
const f1 = safeEval(new Decimal(1));
if (f0 !== null && f1 !== null) {
const slope = f1.minus(f0);
if (!slope.isZero()) {
const root = f0.neg().div(slope);
if (evalF(root).abs().lt(new Decimal("1e-9"))) return qty(root);
const fRoot = safeEval(root);
if (fRoot !== null && fRoot.abs().lt(new Decimal("1e-9"))) return qty(root);
}
}
// Bisection fallback for nonlinear cases
let lo = new Decimal(-1e9);
let hi = new Decimal(1e9);
let flo = evalF(lo);
let fhi = evalF(hi);
if (flo.mul(fhi).gt(0)) throw new EvalError("no solution");
let flo = safeEval(lo);
let fhi = safeEval(hi);
if (flo === null || fhi === null || flo.mul(fhi).gt(0)) {
const probes = [1, 2, 5, 10, 100, 1000, 10000, -1, -2, -5, -10, -100, -1000, -10000];
let found = false;
for (const p of probes) {
const pDec = new Decimal(p);
const fp = safeEval(pDec);
if (fp !== null) {
if (flo === null) { lo = pDec; flo = fp; }
else if (flo.mul(fp).lte(0)) { hi = pDec; fhi = fp; found = true; break; }
else if (fhi === null) { hi = pDec; fhi = fp; }
else if (fhi.mul(fp).lte(0)) { lo = pDec; flo = fp; found = true; break; }
}
}
if (!found) throw new EvalError("no solution");
}
for (let i = 0; i < 100; i++) {
const mid = lo.plus(hi).div(2);
if (hi.minus(lo).abs().lt(new Decimal("1e-10"))) return qty(mid);
const fm = evalF(mid);
if (flo.mul(fm).lte(0)) { hi = mid; fhi = fm; }
const fm = safeEval(mid);
if (fm === null) {
lo = mid.plus("1e-9");
continue;
}
if (flo!.mul(fm).lte(0)) { hi = mid; fhi = fm; }
else { lo = mid; flo = fm; }
}
return qty(lo.plus(hi).div(2));

View file

@ -74,6 +74,9 @@ export function formatValue(v: Value, s: EngineSettings): string {
// quantity
if (v.repr === "hex" || v.repr === "binary" || v.repr === "octal") {
if (v.value.abs().gte(new Decimal("1e10000"))) {
return formatNumber(v.value, s);
}
const int = v.value.toDecimalPlaces(0, Decimal.ROUND_HALF_UP);
const neg = int.isNegative();
const abs = int.abs();