mirror of
https://github.com/libretro/scummvm.git
synced 2024-12-13 12:39:56 +00:00
Made CineUnpacker::unpack more robust & secure. It shouldn't crash now with any input (Before making reading or writing operations they are checked to be in bounds). Also updated some comments and added some error message(s).
svn-id: r32687
This commit is contained in:
parent
46681407ab
commit
d7d9348243
@ -128,7 +128,7 @@ void CineEngine::readVolCnf() {
|
||||
f.read(buf, packedSize);
|
||||
if (packedSize != unpackedSize) {
|
||||
CineUnpacker cineUnpacker;
|
||||
if (!cineUnpacker.unpack(buf, buf, packedSize)) {
|
||||
if (!cineUnpacker.unpack(buf, packedSize, buf, unpackedSize)) {
|
||||
error("Error while unpacking 'vol.cnf' data");
|
||||
}
|
||||
}
|
||||
@ -226,7 +226,9 @@ byte *readBundleFile(int16 foundFileIdx) {
|
||||
byte *unpackBuffer = (byte *)malloc(partBuffer[foundFileIdx].packedSize);
|
||||
readFromPart(foundFileIdx, unpackBuffer);
|
||||
CineUnpacker cineUnpacker;
|
||||
cineUnpacker.unpack(dataPtr, unpackBuffer, partBuffer[foundFileIdx].packedSize);
|
||||
if (!cineUnpacker.unpack(unpackBuffer, partBuffer[foundFileIdx].packedSize, dataPtr, partBuffer[foundFileIdx].unpackedSize)) {
|
||||
warning("Error unpacking '%s' from bundle file '%s'", partBuffer[foundFileIdx].partName, currentPartName);
|
||||
}
|
||||
free(unpackBuffer);
|
||||
} else {
|
||||
readFromPart(foundFileIdx, dataPtr);
|
||||
|
@ -31,6 +31,10 @@
|
||||
namespace Cine {
|
||||
|
||||
uint32 CineUnpacker::readSource() {
|
||||
if (_src < _srcBegin || _src + 4 > _srcEnd) {
|
||||
_error = true;
|
||||
return 0; // The source pointer is out of bounds, returning a default value
|
||||
}
|
||||
uint32 value = READ_BE_UINT32(_src);
|
||||
_src -= 4;
|
||||
return value;
|
||||
@ -67,7 +71,10 @@ uint16 CineUnpacker::getBits(byte numBits) {
|
||||
}
|
||||
|
||||
void CineUnpacker::unpackRawBytes(uint16 numBytes) {
|
||||
_datasize -= numBytes;
|
||||
if (_dst >= _dstEnd || _dst - numBytes + 1 < _dstBegin) {
|
||||
_error = true;
|
||||
return; // Destination pointer is out of bounds for this operation
|
||||
}
|
||||
while (numBytes--) {
|
||||
*_dst = (byte)getBits(8);
|
||||
--_dst;
|
||||
@ -75,21 +82,33 @@ void CineUnpacker::unpackRawBytes(uint16 numBytes) {
|
||||
}
|
||||
|
||||
void CineUnpacker::copyRelocatedBytes(uint16 offset, uint16 numBytes) {
|
||||
_datasize -= numBytes;
|
||||
if (_dst + offset >= _dstEnd || _dst - numBytes + 1 < _dstBegin) {
|
||||
_error = true;
|
||||
return; // Destination pointer is out of bounds for this operation
|
||||
}
|
||||
while (numBytes--) {
|
||||
*_dst = *(_dst + offset);
|
||||
--_dst;
|
||||
}
|
||||
}
|
||||
|
||||
bool CineUnpacker::unpack(byte *dst, const byte *src, int srcLen) {
|
||||
_src = src + srcLen - 4;
|
||||
_datasize = readSource(); // Unpacked length in bytes
|
||||
_dst = dst + _datasize - 1;
|
||||
bool CineUnpacker::unpack(const byte *src, uint srcLen, byte *dst, uint dstLen) {
|
||||
// Initialize variables used for detecting errors during unpacking
|
||||
_error = false;
|
||||
_srcBegin = src;
|
||||
_srcEnd = src + srcLen;
|
||||
_dstBegin = dst;
|
||||
_dstEnd = dst + dstLen;
|
||||
|
||||
// Initialize other variables
|
||||
_src = _srcBegin + srcLen - 4;
|
||||
uint32 unpackedLength = readSource(); // Unpacked length in bytes
|
||||
_dst = _dstBegin + unpackedLength - 1;
|
||||
_crc = readSource();
|
||||
_chunk32b = readSource();
|
||||
_crc ^= _chunk32b;
|
||||
do {
|
||||
|
||||
while (_dst >= _dstBegin && !_error) {
|
||||
/*
|
||||
Bits => Action:
|
||||
0 0 => unpackRawBytes(3 bits + 1) i.e. unpackRawBytes(1..9)
|
||||
@ -123,8 +142,8 @@ bool CineUnpacker::unpack(byte *dst, const byte *src, int srcLen) {
|
||||
copyRelocatedBytes(offset, numBytes);
|
||||
}
|
||||
}
|
||||
} while (_datasize > 0 && _src >= src - 4);
|
||||
return _crc == 0;
|
||||
}
|
||||
return !_error && (_crc == 0);
|
||||
}
|
||||
|
||||
} // End of namespace Cine
|
||||
|
@ -40,7 +40,7 @@ namespace Cine {
|
||||
class CineUnpacker {
|
||||
public:
|
||||
/** Returns true if unpacking was successful, otherwise false. */
|
||||
bool unpack(byte *dst, const byte *src, int srcLen);
|
||||
bool unpack(const byte *src, uint srcLen, byte *dst, uint dstLen);
|
||||
private:
|
||||
/** Reads a single big endian 32-bit integer from the source and goes backwards 4 bytes. */
|
||||
uint32 readSource();
|
||||
@ -69,11 +69,17 @@ private:
|
||||
*/
|
||||
void copyRelocatedBytes(uint16 offset, uint16 numBytes);
|
||||
private:
|
||||
int _datasize; //!< Bytes left to write into the unpacked data stream
|
||||
uint32 _crc; //!< Error-detecting code
|
||||
uint32 _crc; //!< Error-detecting code (This should be zero after successful unpacking)
|
||||
uint32 _chunk32b; //!< The current internal 32-bit chunk
|
||||
byte *_dst; //!< Destination buffer pointer
|
||||
const byte *_src; //!< Source buffer pointer
|
||||
byte *_dst; //!< Pointer to the current position in the destination buffer
|
||||
const byte *_src; //!< Pointer to the current position in the source buffer
|
||||
|
||||
// These are used for detecting errors (e.g. out of bounds issues) during unpacking
|
||||
bool _error; //!< Did an error occur during unpacking?
|
||||
const byte *_srcBegin; //!< Source buffer's beginning
|
||||
const byte *_srcEnd; //!< Source buffer's end
|
||||
byte *_dstBegin; //!< Destination buffer's beginning
|
||||
byte *_dstEnd; //!< Destination buffer's end
|
||||
};
|
||||
|
||||
} // End of namespace Cine
|
||||
|
Loading…
Reference in New Issue
Block a user