Skip to content

Commit 85687b5

Browse files
authored
iobuffer: fix overflow OOB read/write bugs (#59728)
maxsize is usually typemax, so need to be careful to not do comparisons after adding to it. Substracting from it should normally be perfectly fine. At worst we should compute a negative amount of space remaining.
1 parent 0e30e0f commit 85687b5

File tree

2 files changed

+8
-5
lines changed

2 files changed

+8
-5
lines changed

base/iobuffer.jl

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,8 @@ end
604604
end
605605
# The fast path here usually checks there is already room, then does nothing.
606606
# When append is true, new data is added after io.size, not io.ptr
607-
existing_space = min(lastindex(io.data), io.maxsize + get_offset(io)) - (io.append ? io.size : io.ptr - 1)
607+
start_offset = io.append ? io.size : io.ptr - 1
608+
existing_space = min(lastindex(io.data) - start_offset, io.maxsize - (start_offset - get_offset(io)))
608609
if existing_space < nshort % Int
609610
# Outline this function to make it more likely that ensureroom inlines itself
610611
return ensureroom_slowpath(io, nshort, existing_space)
@@ -898,12 +899,13 @@ function unsafe_write(to::GenericIOBuffer, p::Ptr{UInt8}, nb::UInt)
898899
append = to.append
899900
ptr = append ? size+1 : to.ptr
900901
data = to.data
901-
to_write = min(nb, (min(Int(length(data))::Int, to.maxsize + get_offset(to)) - ptr + 1) % UInt) % Int
902+
start_offset = ptr - 1
903+
to_write = max(0, min(nb, (min(Int(length(data))::Int - start_offset, to.maxsize - (start_offset - get_offset(to)))) % UInt) % Int)
902904
# Dispatch based on the type of data, to possibly allow using memcpy
903905
_unsafe_write(data, p, ptr, to_write % UInt)
904906
# Update to.size only if the ptr has advanced to higher than
905907
# the previous size. Otherwise, we just overwrote existing data
906-
to.size = max(size, ptr + to_write - 1)
908+
to.size = max(size, start_offset + to_write)
907909
# If to.append, we only update size, not ptr.
908910
if !append
909911
to.ptr = ptr + to_write
@@ -941,7 +943,7 @@ end
941943
ptr = (to.append ? to.size+1 : to.ptr)
942944
# We have just ensured there is room for 1 byte, EXCEPT if we were to exceed
943945
# maxsize. So, we just need to check that here.
944-
if ptr > to.maxsize + get_offset(to)
946+
if ptr - get_offset(to) > to.maxsize
945947
return 0
946948
else
947949
to.data[ptr] = a

base/stream.jl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,8 @@ end
612612
function alloc_request(buffer::IOBuffer, recommended_size::UInt)
613613
ensureroom(buffer, recommended_size)
614614
ptr = buffer.append ? buffer.size + 1 : buffer.ptr
615-
nb = min(length(buffer.data), buffer.maxsize + get_offset(buffer)) - ptr + 1
615+
start_offset = ptr - 1
616+
nb = max(0, min(length(buffer.data) - start_offset, buffer.maxsize - (start_offset - get_offset(buffer))))
616617
return (Ptr{Cvoid}(pointer(buffer.data, ptr)), nb)
617618
end
618619

0 commit comments

Comments
 (0)