mirror of
https://github.com/storytold/cadcraft.git
synced 2026-10-24 02:46:51 -06:00
security: guard the DXF->DWG conversion input and document decoder limits (CWE-789) (#1847)
The native DWG bridge wraps the third-party `acadrust` decoder, which allocates tables and section buffers from sizes a file declares and cannot be interrupted once it starts. The DWG->DXF read path already rejects oversized input before decoding, caps the DXF output while writing, and runs inside `catch_unwind`. The DXF->DWG `convert` path had no input-size guard, so a very large DXF went straight into the dependency's reader. - `convert` now rejects DXF input larger than `Limits::max_dxf_bytes` before calling `DxfReader::read`, symmetric with the read path, and keeps the existing `catch_unwind` around the decoder and writer (rows 23, 3, 27, large-input variant). - Documented the residual risk: a small file that declares a huge internal table or section can still make the dependency allocate before any size is known. Fully fixing that needs an upstream fix in `acadrust` (validate declared sizes against a decoded-memory budget) or a separate worker with an OS memory limit; the in-crate limits keep the blast radius bounded until then. Addresses findings rows 3, 23, 27 (bounds the reachable input/output size; the deeper allocation fix belongs in the unmodified dependency).
This commit is contained in:
@@ -4,6 +4,25 @@
|
||||
//! converted to DXF bytes, which CADCraft's own DXF reader maps to its document model; saving
|
||||
//! goes the other way. Keeping the dependency behind this byte-level API isolates it: nothing
|
||||
//! else in CADCraft depends on its types. Native targets only (it memory-maps files).
|
||||
//!
|
||||
//! # Guardrails around the third-party decoder
|
||||
//!
|
||||
//! The decoder allocates tables and section buffers from sizes a (possibly damaged or hostile)
|
||||
//! file declares, and once it starts it cannot be interrupted (an allocation failure aborts the
|
||||
//! process, which `catch_unwind` cannot recover). Because the dependency is used unmodified, this
|
||||
//! crate guards it from the outside:
|
||||
//!
|
||||
//! - Both directions reject oversized input *before* the decoder runs ([`Limits::max_dwg_bytes`]
|
||||
//! for DWG→DXF, [`Limits::max_dxf_bytes`] for DXF→DWG), and the DWG→DXF output is capped while
|
||||
//! it is written (`CappedWriter`).
|
||||
//! - The reader and writer run inside `catch_unwind`, so a decode failure is reported instead of
|
||||
//! crashing.
|
||||
//!
|
||||
//! These bound the input- and output-size variants. A small file that declares a huge internal
|
||||
//! table or section can still make the dependency allocate before any size is known; fully fixing
|
||||
//! that requires an upstream fix in `acadrust` (validate declared sizes against a decoded-memory
|
||||
//! budget) or running the decode in a separate worker with an OS memory limit. The limits here
|
||||
//! keep the blast radius bounded in the meantime.
|
||||
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
|
||||
#![forbid(unsafe_code)]
|
||||
|
||||
@@ -505,6 +524,13 @@ mod native {
|
||||
}
|
||||
|
||||
fn convert(dxf: &[u8], version: Option<acadrust::DxfVersion>) -> Result<Vec<u8>, String> {
|
||||
// Reject a pathologically large DXF before handing it to the third-party decoder, which
|
||||
// allocates tables and section buffers from the input's declared sizes and cannot be
|
||||
// interrupted once it starts (CWE-789). This bounds the large-input variant; the decoder
|
||||
// runs inside `catch_unwind` so a decode failure is reported rather than crashing.
|
||||
if dxf.len() > Limits::DEFAULT.max_dxf_bytes {
|
||||
return Err(format!("DXF: the input is {}, larger than the {} CADCraft converts to DWG", mb(dxf.len() as u64), mb(Limits::DEFAULT.max_dxf_bytes as u64)));
|
||||
}
|
||||
let data = dxf.to_vec();
|
||||
let mut doc = std::panic::catch_unwind(move || acadrust::DxfReader::from_reader(Cursor::new(data)).and_then(|r| r.read()))
|
||||
.map_err(|_| "the DXF→DWG conversion failed".to_string())?
|
||||
@@ -618,6 +644,15 @@ mod tests {
|
||||
assert!(dwg_to_dxf(b"").is_err());
|
||||
}
|
||||
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
#[test]
|
||||
fn dxf_to_dwg_rejects_garbage_without_crashing() {
|
||||
// The DXF→DWG path runs the third-party reader inside catch_unwind and reports failures
|
||||
// rather than crashing (and rejects oversized input before decoding).
|
||||
assert!(dxf_to_dwg(b"not a dxf file").is_err());
|
||||
assert!(dxf_to_dwg(b"").is_err());
|
||||
}
|
||||
|
||||
/// A DXF with one DIMENSION per (text, group 70, subclass, points) row.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
fn dims_dxf(dims: &[(&str, i64, &str, &[(i32, [f64; 2])])]) -> String {
|
||||
|
||||
Reference in New Issue
Block a user