Fixed OOB read & write, crash, and inconsistencies

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-10-28 21:44:47 +01:00
parent db967ecdee
commit cb9185d65c
8 changed files with 60 additions and 13 deletions
+3
View File
@@ -18,3 +18,6 @@ Trash/
*.7z
*.zip
/spec*
# Security test suite
SecurityTests/
+6 -5
View File
@@ -47,8 +47,8 @@ Procedure.i MsgPackCreate(InitialSize.i, GrowthIncrement.i = 1)
*ReturnedValue\BufferOffset = 0
*ReturnedValue\BufferSize = InitialSize
*ReturnedValue\BufferGrowthIncrement = GrowthIncrement
*ReturnedValue\DataFlags = 0
*ReturnedValue\DataFlags = #_MsgPack_DefaultDataFlags
*ReturnedValue\Buffer = AllocateMemory(InitialSize)
If Not *ReturnedValue\Buffer
Debug("Failed to allocate memory for buffer !")
@@ -85,11 +85,11 @@ Procedure.i MsgPackWrapMsgBuffer(*Buffer, BufferSize.i, GiveOwnership.b = #False
*ReturnedValue\BufferOffset = 0
*ReturnedValue\BufferSize = BufferSize
*ReturnedValue\BufferGrowthIncrement = GrowthIncrement
*ReturnedValue\DataFlags = 0
*ReturnedValue\DataFlags = #_MsgPack_DefaultDataFlags
*ReturnedValue\Buffer = *Buffer
If Not GiveOwnership
*ReturnedValue\DataFlags = #MsgPack_DataFlags_CannotGrow | #MsgPack_DataFlags_NoBufferFree
*ReturnedValue\DataFlags = *ReturnedValue\DataFlags | #MsgPack_DataFlags_CannotGrow | #MsgPack_DataFlags_NoBufferFree
EndIf
ProcedureReturn *ReturnedValue
@@ -146,7 +146,8 @@ Procedure.MsgPack_ErrorCode MsgPackGrow(*MsgPackData.MsgPackData, DesiredSizeInc
EndIf
; `DesiredSizeIncrease` is positive, so a new size that isn't bigger can only come from an overflow.
If NewSize <= *MsgPackData\BufferSize
; Sizes that don't fit in a `.i` would also be truncated by `ReAllocateMemory()` on x86.
If NewSize <= *MsgPackData\BufferSize Or NewSize > #_MsgPack_MaxIntegerValue
DebuggerError("We had an overflow in one of the growth values, the calculation is messed up !")
*MsgPackData\LastError = #MsgPack_Error_ValueOverflow
ProcedureReturn #MsgPack_Error_ValueOverflow
+9
View File
@@ -122,6 +122,15 @@ EndProcedure
Procedure.MsgPack_ErrorCode _MsgPackWriteSizedField(*MsgPackData.MsgPackData, FormatCode.MsgPack_FormatCode, SizeFieldByteLength.a, *Data, DataSize.q)
Protected ErrorCode.MsgPack_ErrorCode
CompilerIf Not #MsgPack_DisableNullChecks
; Checked before the header is written, so nothing is left half-written.
If *MsgPackData And DataSize > 0 And Not *Data
DebuggerError("A #Null data buffer was passed !")
*MsgPackData\LastError = #MsgPack_Error_NullPointerGiven
ProcedureReturn #MsgPack_Error_NullPointerGiven
EndIf
CompilerEndIf
ErrorCode = _MsgPackWriteSizedFieldHeader(*MsgPackData, FormatCode, SizeFieldByteLength, DataSize)
If ErrorCode <> #MsgPack_Error_Success
ProcedureReturn ErrorCode
+9
View File
@@ -51,6 +51,12 @@ CompilerEndIf
; Used to represent any size that couldn't be calculated/derived/...
#MsgPack_InvalidSize = -1
; Biggest value a `.i` can hold, used to reject sizes that would be truncated on x86.
CompilerIf #PB_Compiler_Processor = #PB_Processor_x86
#_MsgPack_MaxIntegerValue = $7FFFFFFF
CompilerElse
#_MsgPack_MaxIntegerValue = $7FFFFFFFFFFFFFFF
CompilerEndIf
; ------------------------------------------------------------------------------
@@ -188,6 +194,9 @@ EnumerationBinary _MsgPack_DataFlags
#MsgPack_DataFlags_CanProvideVals = %0000000010000000
EndEnumeration
; Flags set on every new `MsgPackData`, so all reads and writes move the offset.
#_MsgPack_DefaultDataFlags = #MsgPack_DataFlags_MoveOffsetAfterRead | #MsgPack_DataFlags_MoveOffsetAfterWrite
EnumerationBinary _MsgPack_ExaminationOptions
; If set, we don't seek the end of the examined map/array.
#MsgPack_ExaminationOptions_NoEndSeeking = %0000000000000001
+7
View File
@@ -58,6 +58,13 @@ Procedure.MsgPack_ErrorCode _MsgPackWriteExtField(*MsgPackData.MsgPackData, Form
DebuggerError("A #Null MsgPackData pointer was passed !")
ProcedureReturn #MsgPack_Error_NullPointerGiven
EndIf
; Checked before the header is written, so nothing is left half-written.
If DataSize > 0 And Not *Data
DebuggerError("A #Null data buffer was passed !")
*MsgPackData\LastError = #MsgPack_Error_NullPointerGiven
ProcedureReturn #MsgPack_Error_NullPointerGiven
EndIf
CompilerEndIf
If SizeFieldByteLength = 0
+10 -4
View File
@@ -51,7 +51,7 @@ Macro _MsgPackGetFieldDynamicSize_Within(MsgPackData, Mask) : (PeekA(MsgPackData
; Applies to: Str8, Bin8, Ext8
; Remark: Doesn't apply to Int/UInt variants since those have their own function for extracting the value.
Procedure.q _MsgPackGetFieldDynamicSize_1After(*MsgPackData.MsgPackData)
If Not _MsgPackHasSpaceLeft(*MsgPackData, *MsgPackData\BufferSize + 1)
If Not _MsgPackHasSpaceLeft(*MsgPackData, 1 + 1)
DebuggerWarning("We have less than 1 byte to read the field's dynamic size !")
ProcedureReturn #MsgPack_InvalidSize
EndIf
@@ -63,7 +63,7 @@ EndProcedure
; Applies to: Str16, Bin16, Ext16, Array16, Map16
; Remark: Doesn't apply to Int/UInt variants since those have their own function for extracting the value.
Procedure.q _MsgPackGetFieldDynamicSize_2After(*MsgPackData.MsgPackData)
If Not _MsgPackHasSpaceLeft(*MsgPackData, *MsgPackData\BufferSize + 2)
If Not _MsgPackHasSpaceLeft(*MsgPackData, 1 + 2)
DebuggerWarning("We have less than 2 bytes to read the field's dynamic size !")
ProcedureReturn #MsgPack_InvalidSize
EndIf
@@ -75,12 +75,12 @@ EndProcedure
; Applies to: Str32, Bin32, Ext32, Array32, Map32
; Remark: Doesn't apply to Int/UInt variants since those have their own function for extracting the value.
Procedure.q _MsgPackGetFieldDynamicSize_4After(*MsgPackData.MsgPackData)
If Not _MsgPackHasSpaceLeft(*MsgPackData, *MsgPackData\BufferSize + 4)
If Not _MsgPackHasSpaceLeft(*MsgPackData, 1 + 4)
DebuggerWarning("We have less than 4 bytes to read the field's dynamic size !")
ProcedureReturn #MsgPack_InvalidSize
EndIf
ProcedureReturn EndianSwapL(PeekL(*MsgPackData\Buffer + *MsgPackData\BufferOffset + 1))
ProcedureReturn EndianSwapL(PeekL(*MsgPackData\Buffer + *MsgPackData\BufferOffset + 1)) & $FFFFFFFF
EndProcedure
; Returns a given format code's spacing info based on itself and the data in the given MsgPack buffer.
@@ -98,6 +98,12 @@ Procedure _MsgPackGetFieldSpacingInfo(*MsgPackData.MsgPackData, ExaminedDataType
ProcedureReturn #False
EndIf
; The "within" sizes are read from the format code, which must be in the buffer.
If _MsgPackIsAtTheEnd(*MsgPackData)
DebuggerWarning("End of buffer reached, cannot read the field's dynamic size !")
ExaminedDataType = #MsgPack_FormatCode_Invalid
EndIf
Select ExaminedDataType
Case #MsgPack_FormatCode_FixIntPositive:
If *VariableSizeQ <> #Null
+12 -4
View File
@@ -39,16 +39,24 @@ XIncludeFile "./_Utils.pbi"
; exactly the missing amount if needed.
; Sets the structure's error code and returns it.
; FIXME: Move elsewhere since it's used in multiple places !
Procedure.MsgPack_ErrorCode _MsgPackReserveWrite(*MsgPackData.MsgPackData, Size.i)
Protected MissingSize.i
Procedure.MsgPack_ErrorCode _MsgPackReserveWrite(*MsgPackData.MsgPackData, Size.q)
Protected MissingSize.q
CompilerIf Not #MsgPack_DisableNullChecks
If Not *MsgPackData
DebuggerError("A #Null MsgPackData pointer was passed !")
ProcedureReturn #MsgPack_Error_NullPointerGiven
EndIf
CompilerEndIf
; `Size` is a quad so sized fields up to 4 GiB can't be truncated, but the end
; offset must still fit in a `.i`, which isn't the case on x86 past 2 GiB.
If Size < 0 Or Size > #_MsgPack_MaxIntegerValue - *MsgPackData\BufferOffset
DebuggerWarning("Requested size can't be addressed: " + Str(Size))
*MsgPackData\LastError = #MsgPack_Error_BinaryDataSizeIsTooBig
ProcedureReturn #MsgPack_Error_BinaryDataSizeIsTooBig
EndIf
MissingSize = (*MsgPackData\BufferOffset + Size) - *MsgPackData\BufferSize
If MissingSize > 0
; The structure's error code field is set by the called function !
@@ -39,6 +39,8 @@ PokeA(*MyBuffer + 1, $C1) ; Invalid
*MsgPack = MsgPackWrapMsgBuffer(*MyBuffer, MemorySize(*MyBuffer))
AssertEqualsA(*MsgPack\BufferOffset, 0, "[False] Offset is 0")
AssertIsTrue(Bool(_MsgPackHasFlag(*MsgPack, #MsgPack_DataFlags_MoveOffsetAfterRead)), "[False] Move after read flag is set by default")
*MsgPack\DataFlags = *MsgPack\DataFlags & (~#MsgPack_DataFlags_MoveOffsetAfterRead)
AssertEqualsA(MsgPackGetFormatCode(*MsgPack), #MsgPack_FormatCode_False, "[False] First format code is false")
AssertEqualsA(MsgPackReadBoolean(*MsgPack), #False, "[False] Boolean literal getter returns #False")
AssertEqualsA(*MsgPack\BufferOffset, 0, "[False] Offset is still 0, we'll change the flag")
@@ -63,6 +65,8 @@ PokeA(*MyBuffer + 1, $C1) ; Invalid
*MsgPack = MsgPackWrapMsgBuffer(*MyBuffer, MemorySize(*MyBuffer))
AssertEqualsA(*MsgPack\BufferOffset, 0, "[True] Offset is 0")
AssertIsTrue(Bool(_MsgPackHasFlag(*MsgPack, #MsgPack_DataFlags_MoveOffsetAfterRead)), "[True] Move after read flag is set by default")
*MsgPack\DataFlags = *MsgPack\DataFlags & (~#MsgPack_DataFlags_MoveOffsetAfterRead)
AssertEqualsA(MsgPackGetFormatCode(*MsgPack), #MsgPack_FormatCode_True, "[True] First format code is false")
AssertEqualsA(MsgPackReadBoolean(*MsgPack), #True, "[True] Boolean literal getter returns #True")
AssertEqualsA(*MsgPack\BufferOffset, 0, "[True] Offset is still 0, we'll change the flag")