From 04407b47a39a000118fef0869da1b85ae55ac8bc Mon Sep 17 00:00:00 2001 From: Nathan Zimmerberg <39104088+nhz2@users.noreply.github.com> Date: Sun, 1 Sep 2024 16:04:37 -0400 Subject: [PATCH] Add extra bounds checking (#4) * add extra bounds checking * update CI --- .github/workflows/CI.yml | 30 ++++++++++++------------ src/stream.jl | 49 ++++++++++++++++++++++++---------------- 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 69d40f5..0f2018a 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -16,33 +16,33 @@ jobs: fail-fast: false matrix: version: - - '1.6' - '1' - - '~1.11.0-0' + - 'pre' os: - ubuntu-latest - - macOS-13 # intel - - macOS-14 # arm - windows-latest arch: - 'x64' - 'x86' - - 'aarch64' - exclude: + include: - os: ubuntu-latest - arch: aarch64 - - os: windows-latest - arch: aarch64 - - os: macOS-13 + version: '1.6' + arch: x64 + - os: ubuntu-latest + version: '1.6' arch: x86 - os: macOS-13 - arch: aarch64 - - os: macOS-14 - arch: x86 - - os: macOS-14 + version: '1' + arch: x64 + - os: macOS-13 + version: 'pre' arch: x64 - os: macOS-14 - version: '1.6' + version: '1' + arch: aarch64 + - os: macOS-14 + version: 'pre' + arch: aarch64 steps: - uses: actions/checkout@v4 - uses: julia-actions/setup-julia@v2 diff --git a/src/stream.jl b/src/stream.jl index 68c2f55..8ecb35e 100644 --- a/src/stream.jl +++ b/src/stream.jl @@ -39,6 +39,7 @@ Base.@kwdef mutable struct StreamState len::UInt32=0 dist::UInt32=0 lit::UInt8=0 + copy_len::UInt16=0 deflate64::Bool in_final_block::Bool=false nlit::UInt16=0 @@ -64,6 +65,7 @@ function reset!(s::StreamState) s.len = 0 s.dist = 0 s.lit = 0 + s.copy_len = 0 s.in_final_block = false s.nlit = 0 s.ndist = 0 @@ -107,25 +109,25 @@ function main_run!(input::TranscodingStreams.Memory, output::TranscodingStreams. # @info "after reading" # @show Δin Δout s.bp s.bits_left elseif s.mode == COPY_OUT - # @info s.len + # @info s.copy_len # @info "before refund" # @show Δin Δout s.bp s.bits_left Δin += refund_in_buf!(s) # @info "after refund" # @show Δin Δout s.bp s.bits_left @assert iszero(s.bp) - @assert !signbit(Δin) + !signbit(Δin) || error("internal error copying from input") in_left = in_size - Δin out_margin = out_size - Δout - n_copy = min(out_margin, Int64(s.len), in_left) + n_copy = UInt16(min(out_margin, Int64(s.copy_len), in_left)) # @show n_copy - !signbit(n_copy) || error("internal error copying from input, this should never happen") - unsafe_copyto!(output.ptr+Δout, input.ptr+Δin, n_copy) + unsafe_copyto!(output.ptr+Δout, input.ptr+Δin, Int(n_copy)) copy_from_input!(s, input.ptr+Δin, n_copy) - s.len -= n_copy + # These cannot overflow out_size because of min check + s.copy_len -= n_copy Δin += n_copy Δout += n_copy - if iszero(s.len) + if iszero(s.copy_len) if s.in_final_block s.mode = DONE else @@ -140,31 +142,34 @@ function main_run!(input::TranscodingStreams.Memory, output::TranscodingStreams. end elseif s.mode == RUN_OP out_margin = out_size - Δout + !signbit(out_margin) || error("internal error copying from output") # length distance copy n_copy = min(out_margin, Int64(s.len)) - !signbit(n_copy) || error("internal error copying from output, this should never happen") copy_from_output!(output.ptr+Δout, s, n_copy, s.dist) # this can error if s.dist goes before the start of the out buffer. s.len -= n_copy - Δout += n_copy + Δout += n_copy # This cannot overflow out_size because of min check if iszero(s.len) s.mode = READ_BITS else Δin += refund_in_buf!(s) + !signbit(Δin) || error("internal error refunding to input after partial RUN_OP") return :output, Δin, Δout end elseif s.mode == WRITE_LIT out_margin = out_size - Δout if out_margin < 1 Δin += refund_in_buf!(s) + !signbit(Δin) || error("internal error refunding to input after partial WRITE_LIT") return :output, Δin, Δout else unsafe_store!(output.ptr+Δout, s.lit) - Δout += 1 copy_one_byte!(s, s.lit) + Δout += 1 s.mode = READ_BITS end elseif s.mode == DONE Δin += refund_in_buf!(s) + !signbit(Δin) || error("internal error refunding to input after DONE") return :done, Δin + !iszero(s.bp), Δout else @assert false "unreachable" @@ -216,7 +221,7 @@ function read_input_bits!(s::StreamState)::Bool if len ⊻ nlen != 0xffff throw(DecompressionError("corrupted copy lengths")) end - s.len = len + s.copy_len = len end elseif s.in_mode == NUM_CODES let s=s @@ -448,25 +453,29 @@ end """ copy `n_copy` bytes from `in_ptr` into the out buffer. """ -function copy_from_input!(s::StreamState, in_ptr::Ptr{UInt8}, n_copy::Int64)::Nothing - @assert n_copy ∈ (0:BUFFER_SIZE) +function copy_from_input!(s::StreamState, in_ptr::Ptr{UInt8}, u16_n_copy::UInt16)::Nothing + n_copy = Int64(u16_n_copy) out_buf = s.out_buf out_offset = s.out_offset + # this addition cannot overflow because n_copy and out_offset are ∈ 0:BUFFER_SIZE s.out_full |= (out_offset + n_copy ≥ BUFFER_SIZE) - n_copy_next = min(BUFFER_SIZE - out_offset, n_copy) + # n_copy_next ∈ 0:n_copy because out_offset < BUFFER_SIZE and n_copy ≥ 0 + n_copy_next = min(BUFFER_SIZE - out_offset, n_copy)::Int64 + # This is inbounds because the min above. GC.@preserve out_buf unsafe_copyto!(pointer(out_buf, 1 + out_offset), in_ptr, n_copy_next) - out_offset += n_copy_next%UInt16 if n_copy_next < n_copy + # wrap around and write any extra data to the start of out_buf n_copy_start = n_copy - n_copy_next - GC.@preserve out_buf unsafe_copyto!(pointer(out_buf, 1 + out_offset), in_ptr + n_copy_next, n_copy_start) - out_offset += n_copy_start%UInt16 + # This is inbounds because n_copy_next ∈ 0:n_copy , so n_copy_start ∈ 0:n_copy, and + # n_copy ∈ 0:BUFFER_SIZE as a precondition + GC.@preserve out_buf unsafe_copyto!(pointer(out_buf, 1), in_ptr + n_copy_next, n_copy_start) end - s.out_offset = out_offset + s.out_offset += u16_n_copy nothing end """ -copy `n_copy` bytes from `dist` back in the out buffer in to `out_ptr` and the out buffer. +copy `n_copy` bytes from `dist` back in the out buffer into `out_ptr` and the out buffer. this can error if `dist` goes before the start of the out buffer. """ function copy_from_output!(out_ptr::Ptr{UInt8}, s::StreamState, n_copy::Int64, dist::UInt32)::Nothing @@ -486,6 +495,8 @@ end write one byte into the out buffer. """ function copy_one_byte!(s::StreamState, lit::UInt8)::Nothing + # this should always be inbounds because s.out_offset is a UInt16 + # and s.out_buf is length 2^16 @inbounds s.out_buf[begin + s.out_offset] = lit s.out_offset += UInt16(1) s.out_full |= iszero(s.out_offset)