fix(figma): codex review round 1 — hardening + ZIP entrypoint
Addresses the first codex review of the binary `.fig` parser. BLOCKs: - kiwi: 64-bit varint now uses Kiwi's terminal-byte rule (eight 7-bit groups then a final full-8-bit byte) — the old `& 127` mask on every byte corrupted u64 values above 2^56. - kiwi: an invalid schema definition kind (> 2) is now rejected instead of silently treated as a message. - kiwi: array decode rejects a length exceeding the buffer size — guards against a hostile zero-byte-element array spinning the decode loop billions of times. - zip_reader: aggregate 2 GiB decompression budget + 10k entry cap on top of the existing per-entry limit (zip-bomb defence). CONCERNs: - detect_kind now recognises the `PK\x03\x04` ZIP magic as Binary — the common Figma export form (`canvas.fig` + `images/` in a ZIP) was being rejected before container.rs could unwrap it. - resolve_style_references now also resolves style refs inside instance `symbolData.symbolOverrides` entries. - kiwi: enum field type codes are no longer resolved (unused; kiwi writes 0) so a stray code can't reject a valid schema. Plus a zip-wrapped end-to-end test + the misleading zstd test rename. op-figma 84 tests green (+1); clean build. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This commit is contained in:
parent
155f9daee7
commit
ac63c701e6
|
|
@ -253,6 +253,53 @@ fn parses_a_full_binary_fig_into_a_document() {
|
|||
}
|
||||
}
|
||||
|
||||
/// Wrap a payload as `canvas.fig` in a minimal stored (uncompressed)
|
||||
/// ZIP archive — the common Figma export form.
|
||||
fn wrap_in_zip(canvas: &[u8]) -> Vec<u8> {
|
||||
const LFH: u32 = 0x0403_4b50;
|
||||
const CDFH: u32 = 0x0201_4b50;
|
||||
const EOCD: u32 = 0x0605_4b50;
|
||||
let name = b"canvas.fig";
|
||||
let mut z = Vec::new();
|
||||
let local_off = 0u32;
|
||||
z.extend_from_slice(&LFH.to_le_bytes());
|
||||
z.extend_from_slice(&[20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
|
||||
z.extend_from_slice(&(canvas.len() as u32).to_le_bytes());
|
||||
z.extend_from_slice(&(canvas.len() as u32).to_le_bytes());
|
||||
z.extend_from_slice(&(name.len() as u16).to_le_bytes());
|
||||
z.extend_from_slice(&[0, 0]);
|
||||
z.extend_from_slice(name);
|
||||
z.extend_from_slice(canvas);
|
||||
let cd_off = z.len() as u32;
|
||||
z.extend_from_slice(&CDFH.to_le_bytes());
|
||||
z.extend_from_slice(&[20, 0, 20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
|
||||
z.extend_from_slice(&(canvas.len() as u32).to_le_bytes());
|
||||
z.extend_from_slice(&(canvas.len() as u32).to_le_bytes());
|
||||
z.extend_from_slice(&(name.len() as u16).to_le_bytes());
|
||||
z.extend_from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
|
||||
z.extend_from_slice(&local_off.to_le_bytes());
|
||||
z.extend_from_slice(name);
|
||||
let cd_size = z.len() as u32 - cd_off;
|
||||
z.extend_from_slice(&EOCD.to_le_bytes());
|
||||
z.extend_from_slice(&[0, 0, 0, 0, 1, 0, 1, 0]);
|
||||
z.extend_from_slice(&cd_size.to_le_bytes());
|
||||
z.extend_from_slice(&cd_off.to_le_bytes());
|
||||
z.extend_from_slice(&[0, 0]);
|
||||
z
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_a_zip_wrapped_fig() {
|
||||
let bare = build_fig(&build_schema(), &build_data());
|
||||
let zipped = wrap_in_zip(&bare);
|
||||
// The ZIP form must be recognised + routed through the parser.
|
||||
let import = parse_fig_binary(&zipped, "Zipped", FigLayoutMode::OpenPencil)
|
||||
.expect("zip-wrapped .fig parses");
|
||||
let pages = import.document.pages.expect("has pages");
|
||||
assert_eq!(pages[0].name, "Page 1");
|
||||
assert_eq!(pages[0].children.len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn document_serializes_to_canonical_json() {
|
||||
let fig = build_fig(&build_schema(), &build_data());
|
||||
|
|
|
|||
|
|
@ -193,10 +193,10 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn zstd_chunk_round_trips() {
|
||||
// A zstd-framed chunk: build one with ruzstd's sibling encoder
|
||||
// is not available, so assert the raw-deflate path here and
|
||||
// trust zstd via the magic-detection unit below.
|
||||
fn deflate_chunk_round_trips() {
|
||||
// Chunks are deflate-compressed here (no zstd encoder in the
|
||||
// dev-deps); the zstd decode path is covered by `ruzstd`'s own
|
||||
// test suite and the magic-detection branch in decompress_chunk.
|
||||
let fig = make_fig_container(&[b"only-one"]);
|
||||
let result = fig_to_binary_parts(&fig).expect("parses");
|
||||
assert_eq!(result.parts.len(), 1);
|
||||
|
|
|
|||
|
|
@ -90,15 +90,22 @@ impl<'a> ByteBuffer<'a> {
|
|||
Ok(((v >> 1) as i32) ^ -((v & 1) as i32))
|
||||
}
|
||||
|
||||
/// LEB128 unsigned 64-bit varint (max 10 bytes).
|
||||
/// Kiwi 64-bit unsigned varint: eight 7-bit groups (56 bits) then a
|
||||
/// final byte contributing all 8 bits — matching `kiwi-schema`.
|
||||
pub fn read_var_uint64(&mut self) -> Result<u64, KiwiError> {
|
||||
let mut value: u64 = 0;
|
||||
let mut shift: u32 = 0;
|
||||
loop {
|
||||
let byte = self.read_byte()?;
|
||||
value |= ((byte & 127) as u64).wrapping_shl(shift);
|
||||
shift += 7;
|
||||
if byte & 128 == 0 || shift >= 70 {
|
||||
if shift < 56 {
|
||||
value |= ((byte & 127) as u64).wrapping_shl(shift);
|
||||
shift += 7;
|
||||
if byte & 128 == 0 {
|
||||
break;
|
||||
}
|
||||
} else {
|
||||
// Final byte — all 8 bits, no continuation.
|
||||
value |= (byte as u64).wrapping_shl(shift);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
|
@ -208,7 +215,8 @@ pub fn decode_binary_schema(bytes: &[u8]) -> Result<Schema, KiwiError> {
|
|||
let kind = match bb.read_byte()? {
|
||||
0 => DefKind::Enum,
|
||||
1 => DefKind::Struct,
|
||||
_ => DefKind::Message,
|
||||
2 => DefKind::Message,
|
||||
other => return Err(KiwiError::BadType(format!("definition kind {other}"))),
|
||||
};
|
||||
let field_count = bb.read_var_uint()?;
|
||||
let mut fields = Vec::with_capacity(field_count as usize);
|
||||
|
|
@ -227,7 +235,12 @@ pub fn decode_binary_schema(bytes: &[u8]) -> Result<Schema, KiwiError> {
|
|||
for (name, kind, fields) in raw {
|
||||
let mut resolved = Vec::with_capacity(fields.len());
|
||||
for (field_name, type_code, is_array, value) in fields {
|
||||
let type_name = if type_code < 0 {
|
||||
// Enum fields carry only a name + ordinal; their encoded
|
||||
// type code is unused (kiwi writes 0). Skip resolution so
|
||||
// a stray code never rejects an otherwise-valid schema.
|
||||
let type_name = if kind == DefKind::Enum {
|
||||
String::new()
|
||||
} else if type_code < 0 {
|
||||
let idx = !type_code;
|
||||
NATIVE_TYPES
|
||||
.get(idx as usize)
|
||||
|
|
@ -413,6 +426,13 @@ impl<'a, 'b> Decoder<'a, 'b> {
|
|||
return Ok(FigValue::Bytes(self.bb.read_byte_array()?));
|
||||
}
|
||||
let n = self.bb.read_var_uint()?;
|
||||
// A real array cannot have more elements than the buffer
|
||||
// has bytes — rejects a hostile length that would spin the
|
||||
// decode loop billions of times (e.g. an array of a
|
||||
// zero-byte struct type).
|
||||
if n as usize > self.bb.data.len() {
|
||||
return Err(KiwiError::OutOfBounds);
|
||||
}
|
||||
let mut items = Vec::with_capacity(n.min(1 << 20) as usize);
|
||||
for _ in 0..n {
|
||||
items.push(self.decode_type(&field.type_name, depth + 1)?);
|
||||
|
|
|
|||
|
|
@ -56,10 +56,17 @@ pub enum FigFileKind {
|
|||
/// applies. Returns `Unknown` for empty input.
|
||||
pub fn detect_kind(bytes: &[u8]) -> FigFileKind {
|
||||
// Figma's binary container starts with the literal "fig-kiwi" — a
|
||||
// 8-byte ASCII magic followed by the Zstd-compressed payload.
|
||||
// 8-byte ASCII magic followed by the chunk payload.
|
||||
if bytes.len() >= 8 && &bytes[0..8] == b"fig-kiwi" {
|
||||
return FigFileKind::Binary;
|
||||
}
|
||||
// The common export form wraps the container in a ZIP archive
|
||||
// (`canvas.fig` + `images/*`); the ZIP local-file-header magic
|
||||
// `PK\x03\x04` marks it. A non-fig ZIP simply fails later with a
|
||||
// "no canvas.fig" error.
|
||||
if bytes.len() >= 4 && bytes[0..4] == [0x50, 0x4b, 0x03, 0x04] {
|
||||
return FigFileKind::Binary;
|
||||
}
|
||||
// Clipboard JSON starts with `{` and contains the FIGMA_DOCUMENT
|
||||
// marker within the first KB. Cheap prefix check first.
|
||||
if let Some(first_non_ws) = bytes.iter().find(|b| !b.is_ascii_whitespace()) {
|
||||
|
|
|
|||
|
|
@ -75,6 +75,17 @@ pub fn resolve_style_references(node_changes: &mut [FigValue]) {
|
|||
}
|
||||
for nc in node_changes.iter_mut() {
|
||||
resolve_on_node(nc, &style_map);
|
||||
// Resolve style refs inside instance symbol overrides too.
|
||||
if let Some(mut symbol_data) = nc.get("symbolData").cloned() {
|
||||
if let Some(overrides) = symbol_data.get_array("symbolOverrides") {
|
||||
let mut resolved: Vec<FigValue> = overrides.to_vec();
|
||||
for ov in &mut resolved {
|
||||
resolve_on_node(ov, &style_map);
|
||||
}
|
||||
symbol_data.set("symbolOverrides", FigValue::Array(resolved));
|
||||
nc.set("symbolData", symbol_data);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -38,6 +38,10 @@ const CDFH_SIG: u32 = 0x0201_4b50;
|
|||
const LFH_SIG: u32 = 0x0403_4b50;
|
||||
/// Per-entry decompressed ceiling — zip-bomb defence (512 MiB).
|
||||
const MAX_ENTRY_SIZE: usize = 512 * 1024 * 1024;
|
||||
/// Aggregate decompressed ceiling across the whole archive (2 GiB).
|
||||
const MAX_TOTAL_SIZE: usize = 2 * 1024 * 1024 * 1024;
|
||||
/// Cap on the number of archive entries (many-tiny-files zip bomb).
|
||||
const MAX_ENTRIES: usize = 10_000;
|
||||
|
||||
fn u16_le(b: &[u8], off: usize) -> Option<u16> {
|
||||
b.get(off..off + 2)
|
||||
|
|
@ -72,8 +76,12 @@ pub fn read_zip(buf: &[u8]) -> Result<Vec<ZipEntry>, ZipError> {
|
|||
let eocd = find_eocd(buf).ok_or(ZipError::NotZip)?;
|
||||
let total_entries = u16_le(buf, eocd + 10).ok_or(ZipError::Truncated)? as usize;
|
||||
let cd_offset = u32_le(buf, eocd + 16).ok_or(ZipError::Truncated)? as usize;
|
||||
if total_entries > MAX_ENTRIES {
|
||||
return Err(ZipError::TooLarge);
|
||||
}
|
||||
|
||||
let mut entries = Vec::with_capacity(total_entries);
|
||||
let mut total_size = 0usize;
|
||||
let mut cursor = cd_offset;
|
||||
for _ in 0..total_entries {
|
||||
if u32_le(buf, cursor) != Some(CDFH_SIG) {
|
||||
|
|
@ -93,6 +101,10 @@ pub fn read_zip(buf: &[u8]) -> Result<Vec<ZipEntry>, ZipError> {
|
|||
let name = String::from_utf8_lossy(name_bytes).into_owned();
|
||||
|
||||
let data = read_entry_data(buf, local_off, method, comp_size, uncomp_size)?;
|
||||
total_size = total_size.saturating_add(data.len());
|
||||
if total_size > MAX_TOTAL_SIZE {
|
||||
return Err(ZipError::TooLarge);
|
||||
}
|
||||
entries.push(ZipEntry { name, data });
|
||||
|
||||
cursor += 46 + name_len + extra_len + comment_len;
|
||||
|
|
|
|||
Loading…
Reference in a new issue