glTF2: Fix heap-buffer-overflow in Accessor validation and size calculation (#6473)
* glTF2: Fix heap-buffer-overflow in Accessor validation and size calculation
This patch fixes a heap-buffer-overflow in
`glTF2::Accessor::ExtractData` caused by incorrect bounds validation and
available size reporting.
The vulnerability stemmed from two issues in `glTF2Asset.inl`:
1. **Underestimated validation in `Accessor::Read`**: The logic used
`GetBytesPerComponent() * count` to validate the required buffer
size. This failed to account for the actual `stride`, allowing
accessors to pass validation even if their total footprint
(including stride) exceeded the buffer view.
2. **Incorrect size reporting in `Accessor::GetMaxByteSize`**: The
function returned the total `bufferView->byteLength` while ignoring
the `byteOffset`. Since the accessor data starts at `byteOffset`,
the actual available space is `byteLength - byteOffset`. This led
`ExtractData` to permit reads that extended beyond the end of the
allocated buffer.
Changes:
* Modified `Accessor::Read` to use `GetStride() * count` for length
validation.
* Updated `Accessor::GetMaxByteSize` to correctly return
`bufferView->byteLength - byteOffset` for standard accessors and
`sparse->data.size()` for sparse accessors.
Co-authored-by: CodeMender <codemender-patching@google.com>
Fixes: https://issues.oss-fuzz.com/issues/483102963
* address comment
---------
Co-authored-by: CodeMender <codemender-patching@google.com>
Co-authored-by: Kim Kulling <kimkulling@users.noreply.github.com>
This commit is contained in:
@@ -888,7 +888,9 @@ inline void Accessor::Read(Value &obj, Asset &r) {
|
|||||||
|
|
||||||
if (bufferView) {
|
if (bufferView) {
|
||||||
// Check length
|
// Check length
|
||||||
unsigned long long byteLength = (unsigned long long)GetBytesPerComponent() * (unsigned long long)count;
|
unsigned long long byteLength = count > 0
|
||||||
|
? (unsigned long long)GetStride() * (unsigned long long)(count - 1) + (unsigned long long)GetElementSize()
|
||||||
|
: 0;
|
||||||
|
|
||||||
// handle integer overflow
|
// handle integer overflow
|
||||||
if (byteLength < count) {
|
if (byteLength < count) {
|
||||||
@@ -1004,7 +1006,15 @@ inline size_t Accessor::GetMaxByteSize() {
|
|||||||
if (decodedBuffer)
|
if (decodedBuffer)
|
||||||
return decodedBuffer->byteLength;
|
return decodedBuffer->byteLength;
|
||||||
|
|
||||||
return (bufferView ? bufferView->byteLength : sparse->data.size());
|
if (sparse) {
|
||||||
|
return sparse->data.size();
|
||||||
|
}
|
||||||
|
|
||||||
|
if (bufferView) {
|
||||||
|
return byteOffset <= bufferView->byteLength ? bufferView->byteLength - byteOffset : 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
template <class T>
|
template <class T>
|
||||||
|
|||||||
Reference in New Issue
Block a user