Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/md/ansi_renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,14 +269,21 @@ impl OutputBuffer {
if self.oom {
return;
}
// Vec::extend aborts on OOM under the global mimalloc allocator.
if self.list.try_reserve(data.len()).is_err() {
self.oom = true;
return;
}
self.list.extend_from_slice(data);
}

fn write_byte(&mut self, b: u8) {
if self.oom {
return;
}
if self.list.try_reserve(1).is_err() {
self.oom = true;
return;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
self.list.push(b);
}
}
Expand Down Expand Up @@ -316,7 +323,10 @@ impl<'a> AnsiRenderer<'a> {
last_was_newline: true,
blank_emitted: false,
};
r.out.list.reserve(src_text.len() + src_text.len() / 2);
// The output is usually ~1.5x the input; this is only a throughput
// hint, so on failure fall back to the incremental `try_reserve`s in
// `write` instead of aborting.
let _ = r.out.list.try_reserve(src_text.len() + src_text.len() / 2);
Comment thread
robobun marked this conversation as resolved.
r
}

Expand Down
12 changes: 8 additions & 4 deletions src/md/blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl Parser<'_> {
p_end: &mut OFF,
pivot_line: &Line,
line: &mut Line,
) -> Result<(), bun_alloc::AllocError> {
) -> Result<(), parser::Error> {
let mut off = off_start;
let mut total_indent: u32 = 0;
let mut n_parents: u32 = 0;
Expand Down Expand Up @@ -692,7 +692,7 @@ impl Parser<'_> {
cur_line_idx: usize,
line_buf: &mut [Line; 2],
line_idx: &mut usize,
) -> Result<(), bun_alloc::AllocError> {
) -> Result<(), parser::Error> {
// Index into line_buf via cur_line_idx instead of taking a `&mut Line`
// parameter, which would alias line_buf.
let line = &mut line_buf[cur_line_idx];
Expand Down Expand Up @@ -832,7 +832,7 @@ impl Parser<'_> {
Ok(())
}

pub fn start_new_block(&mut self, line: &Line) -> Result<(), bun_alloc::AllocError> {
pub fn start_new_block(&mut self, line: &Line) -> Result<(), parser::Error> {
let block_type: BlockType = match line.r#type {
LineType::Hr => BlockType::Hr,
LineType::Atxheader => BlockType::H,
Expand All @@ -847,6 +847,7 @@ impl Parser<'_> {
let cur_len = self.block_bytes.len();
let aligned = (cur_len + align_mask) & !align_mask;
let needed = aligned + size_of::<BlockHeader>();
parser::check_block_bytes_len(needed)?;
self.block_bytes.ensure_total_capacity(needed);
// Zero-fill to `needed`; bytes in [aligned, needed) are immediately
// overwritten by the BlockHeader write below.
Expand Down Expand Up @@ -879,7 +880,7 @@ impl Parser<'_> {
Ok(())
}

pub fn end_current_block(&mut self) -> Result<(), bun_alloc::AllocError> {
pub fn end_current_block(&mut self) -> Result<(), parser::Error> {
if let Some(cb_off) = self.current_block {
// Capture the header fields, drop the &mut borrow, then access
// other &self fields.
Expand Down Expand Up @@ -926,6 +927,9 @@ impl Parser<'_> {
self.current_block_lines.len() * size_of::<VerbatimLine>(),
)
};
// The block's lines land in `block_bytes` too (12 bytes per
// line), not just its header, so this growth needs the same cap.
parser::check_block_bytes_len(self.block_bytes.len() + line_bytes.len())?;
self.block_bytes.extend_from_slice(line_bytes);
self.current_block = None;
}
Expand Down
7 changes: 4 additions & 3 deletions src/md/containers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ impl Parser<'_> {
block_type: BlockType,
data: u32,
flags: u32,
) -> Result<(), AllocError> {
) -> Result<(), parser::Error> {
let align_mask: usize = align_of::<BlockHeader>() - 1;
let cur_len = self.block_bytes.len();
let aligned = (cur_len + align_mask) & !align_mask;
let needed = aligned + size_of::<BlockHeader>();
parser::check_block_bytes_len(needed)?;
self.block_bytes
.reserve(needed.saturating_sub(self.block_bytes.len()));
// Zero-fill to `needed`; bytes in [aligned, needed) are immediately
Expand All @@ -49,7 +50,7 @@ impl Parser<'_> {
Ok(())
}

pub fn enter_child_containers(&mut self, count: u32) -> Result<(), AllocError> {
pub fn enter_child_containers(&mut self, count: u32) -> Result<(), parser::Error> {
let mut i: u32 = self.n_containers - count;
while i < self.n_containers {
// Capture the container fields before calling &mut self methods.
Expand Down Expand Up @@ -100,7 +101,7 @@ impl Parser<'_> {
Ok(())
}

pub fn leave_child_containers(&mut self, keep: u32) -> Result<(), AllocError> {
pub fn leave_child_containers(&mut self, keep: u32) -> Result<(), parser::Error> {
while self.n_containers > keep {
self.n_containers -= 1;
// Capture the container fields before calling &mut self methods.
Expand Down
50 changes: 43 additions & 7 deletions src/md/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl Default for BlockHeader {
}

/// `Parser`'s error type: the union of `{ OutOfMemory, JSError, JSTerminated }`
/// with the parser-specific `{ StackOverflow, InputTooLarge }`.
/// with the parser-specific `{ StackOverflow, InputTooLarge, TooManyBlocks }`.
// (`bun_jsc::JsError` covers the first three, but the md crate sits below
// `bun_jsc` in the layering, so the variants stay flat here.)
pub type Error = ParserError;
Expand All @@ -141,9 +141,12 @@ pub enum ParserError {
JSError,
JSTerminated,
StackOverflow,
/// The input is longer than `OFF::MAX` bytes, so its offsets cannot be
/// represented by the parser's `u32` offset type.
/// The input is longer than [`MAX_INPUT_LEN`], so the parser's `u32`
/// offset arithmetic cannot address it.
InputTooLarge,
/// The document needs more than [`MAX_BLOCK_BYTES`] of block metadata,
/// so the parser's `u32` block offsets cannot address it.
TooManyBlocks,
}

bun_core::oom_from_alloc!(ParserError);
Expand All @@ -154,12 +157,45 @@ impl From<ParserError> for bun_core::Error {
}
}

/// Every offset, mark and span boundary in the parser is an `OFF` (u32), so an
/// input of 2^32 bytes or more cannot be indexed. Callers that size anything
/// from the input length must reject it with this before allocating.
/// The longest fixed lookahead the parser performs from an in-bounds offset:
/// the 9-byte `<![CDATA[` probe in `is_html_block_start_condition`.
const MAX_LOOKAHEAD: OFF = 9;
Comment thread
robobun marked this conversation as resolved.
Outdated

/// The largest input `input_size` accepts. Every offset, mark and span
/// boundary in the parser is an `OFF` (u32), and bounds checks are written as
/// `off + k <= size` for fixed lookaheads `k`, so the input must leave
/// [`MAX_LOOKAHEAD`] bytes of headroom below `OFF::MAX` for that arithmetic
/// never to wrap.
pub const MAX_INPUT_LEN: usize = (OFF::MAX - MAX_LOOKAHEAD) as usize;

/// The most bytes `block_bytes` may hold: block offsets are
/// stored as `u32`s (`BlockHeader.data` links, `Container.block_byte_off`),
/// and each new header is written at the end of `block_bytes` rounded up to
/// its alignment, so the buffer must stop one aligned header short of
/// `OFF::MAX`.
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
pub(crate) const MAX_BLOCK_BYTES: usize =
Comment thread
robobun marked this conversation as resolved.
OFF::MAX as usize - (size_of::<BlockHeader>() + align_of::<BlockHeader>());

/// Rejects growing `block_bytes` to `needed` bytes once the parser's u32
/// block offsets could no longer address it. Every site that grows the
/// buffer (`start_new_block`, `push_container_bytes`, `end_current_block`)
/// checks this before appending.
#[inline]
pub(crate) fn check_block_bytes_len(needed: usize) -> Result<(), ParserError> {
Comment thread
robobun marked this conversation as resolved.
if needed > MAX_BLOCK_BYTES {
return Err(ParserError::TooManyBlocks);
}
Ok(())
}

/// Callers that size anything from the input length must reject oversized
/// inputs with this before allocating.
#[inline]
pub(crate) fn input_size(text: &[u8]) -> Result<OFF, ParserError> {
OFF::try_from(text.len()).map_err(|_| ParserError::InputTooLarge)
if text.len() > MAX_INPUT_LEN {
return Err(ParserError::InputTooLarge);
}
Ok(text.len() as OFF)
}

impl<'a> Parser<'a> {
Expand Down
14 changes: 12 additions & 2 deletions src/runtime/api/MarkdownObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use bun_jsc::{
// thin mod-decl shim, so alias the `root` module (which re-exports BlockType,
// SpanType, TextType, SpanDetail, Renderer, helpers, types, ansi, …) as `md`.
use crate::node::StringOrBuffer;
use bun_md::parser::ParserError;
use bun_md::parser::{MAX_INPUT_LEN, ParserError};
use bun_md::root as md;

// `bun_core::String::create_utf8_for_js` lives in `bun_jsc::bun_string_jsc`
Expand Down Expand Up @@ -54,11 +54,21 @@ fn parser_err_to_js(
ParserError::InputTooLarge => global_this.throw_range_error(
input_len as i64,
RangeErrorOptions {
max: md::types::OFF::MAX as i64,
max: MAX_INPUT_LEN as i64,
field_name: b"input.byteLength",
..Default::default()
},
),
// The document, not the input length, overflowed the parser's u32
// block offsets, so an `input.byteLength` bound would be misleading.
ParserError::TooManyBlocks => global_this
.err(
bun_jsc::ErrCode::OUT_OF_RANGE,
format_args!(
"markdown input requires more block metadata than the parser can address (4 GiB)"
),
)
.throw(),
}
}

Expand Down
50 changes: 32 additions & 18 deletions test/js/bun/md/md-edge-cases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,10 @@ describe("pathological reference definition inputs", () => {
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
timeout: 30_000,
// 220k lines through a debug+ASAN child run close to 30s on a loaded
// runner, which made this the flakiest test in the file; the hard stop
// only has to stay under the test's own 90s timeout.
timeout: 75_000,
killSignal: "SIGKILL",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
Expand Down Expand Up @@ -1190,21 +1193,24 @@ describe("pathological autolink opener inputs", () => {
}, 90_000);
});

describe("inputs of 2^32 bytes or more", () => {
// The parser addresses its input with u32 offsets, so a 2^32-byte input
// cannot be represented and must be rejected with a catchable RangeError by
// every entry point. One subprocess covers all four: a crash must not take
// down the test runner, and one 4 GiB reservation (virtual only, never
// written) keeps this cheap. The SKIP branch covers runners that cannot
// reserve 4 GiB; under ASAN the allocator must be allowed to return null
// for that to surface as a catchable error rather than an abort. The
// explicit timeout is for the debug+ASAN lanes, where a spawned child is
// slow under load (same as the linear-time test above).
test("html, ansi, render and react reject a 2^32-byte input", async () => {
describe("inputs the parser cannot address", () => {
// The parser addresses its input with u32 offsets and probes up to 9 bytes
// past an offset (the `<![CDATA[` check), so everything longer than
// 4294967286 bytes (u32::MAX - 9) must be rejected with a catchable
// RangeError by every entry point: at u32::MAX exactly, the probe's
// `off + 9` would wrap. One subprocess covers the four entry points at
// 2^32 bytes and the first rejected length; both buffers are virtual only
// (never written), so this is cheap. The SKIP branch covers runners that
// cannot reserve the address space; under ASAN the allocator must be
// allowed to return null for that to surface as a catchable error rather
// than an abort. The explicit timeout is for the debug+ASAN lanes, where a
// spawned child is slow under load (same as the linear-time test above).
test("html, ansi, render and react reject inputs past the addressable limit", async () => {
const script = `
let big;
let big, boundary;
try {
big = new Uint8Array(2 ** 32);
boundary = new Uint8Array(2 ** 32 - 1);
} catch {
console.log(JSON.stringify("SKIP"));
process.exit(0);
Expand All @@ -1214,6 +1220,9 @@ describe("inputs of 2^32 bytes or more", () => {
() => Bun.markdown.ansi(big),
() => Bun.markdown.render(big, {}),
() => Bun.markdown.react(big, undefined, { reactVersion: 18 }),
// One past the accepted maximum of 4294967286 from the other side:
// the largest allocatable length that must still be rejected.
() => Bun.markdown.html(boundary),
];
const results = [];
for (const run of runs) {
Expand All @@ -1234,13 +1243,18 @@ describe("inputs of 2^32 bytes or more", () => {
},
stdout: "pipe",
stderr: "inherit",
// An accepted 4 GiB input would take minutes to scan; if a regression
// accepts one of these lengths again, kill the child instead.
timeout: 20_000,
Comment thread
robobun marked this conversation as resolved.
Outdated
killSignal: "SIGKILL",
});
const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
const rangeError =
'RangeError | ERR_OUT_OF_RANGE | The value of "input.byteLength" is out of range. It must be <= 4294967295. Received 4294967296';
expect(["SKIP", [rangeError, rangeError, rangeError, rangeError]]).toContainEqual(
JSON.parse(stdout.trim() || '"NO_OUTPUT"'),
);
const message = (received: number) =>
`RangeError | ERR_OUT_OF_RANGE | The value of "input.byteLength" is out of range. It must be <= 4294967286. Received ${received}`;
expect([
"SKIP",
[message(2 ** 32), message(2 ** 32), message(2 ** 32), message(2 ** 32), message(2 ** 32 - 1)],
]).toContainEqual(JSON.parse(stdout.trim() || '"NO_OUTPUT"'));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
expect(exitCode).toBe(0);
}, 30_000);
});
Loading