db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-03-29 21:47:21 +02:00
Code Issues Releases Activity

Add bufferSize to cvtUInt64FromVarInt128() to further limit reads.

Browse Source 
The current call site, manifestFileUnpack(), does not know the total buffer size but the buffer has always been maintained in memory so there should be no corruption. However, there are upcoming use cases where the buffer will be read from IO, the buffer size will be known, and additional sanity checking on buffer overruns will be valuable.

Also rename params to align better with cvtUInt64ToVarInt128().
This commit is contained in:
David Steele 2022-10-02 17:32:48 -10:00
parent 01b81f9d37
commit ac99201c0e
4 changed files with 33 additions and 25 deletions
src
common/type
convert.cconvert.h
info
manifest.c
test/src/module/common
typeConvertTest.c

27
src/common/type/convert.c

@ -603,30 +603,35 @@ cvtUInt64ToVarInt128(uint64_t value, uint8_t *const buffer, size_t *const buffer
}
uint64_t
cvtUInt64FromVarInt128(const uint8_t *const value, size_t *const valuePos)
cvtUInt64FromVarInt128(const uint8_t *const buffer, size_t *const bufferPos, const size_t bufferSize)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM_P(VOID, value);
FUNCTION_TEST_PARAM_P(UINT64, valuePos);
FUNCTION_TEST_PARAM_P(VOID, buffer);
FUNCTION_TEST_PARAM_P(SIZE, bufferPos);
FUNCTION_TEST_PARAM(SIZE, bufferSize);
FUNCTION_TEST_END();
ASSERT(value != NULL);
ASSERT(valuePos != NULL);
ASSERT(buffer != NULL);
ASSERT(bufferPos != NULL);
// Decode all bytes
uint64_t result = 0;
uint8_t byte;
for (unsigned int valueIdx = 0; valueIdx < CVT_VARINT128_BUFFER_SIZE; valueIdx++)
for (unsigned int bufferIdx = 0; bufferIdx < CVT_VARINT128_BUFFER_SIZE; bufferIdx++)
{
// Error if the buffer position is beyond the buffer size
if (*bufferPos >= bufferSize)
THROW(FormatError, "buffer position is beyond buffer size");
// Get the next encoded byte
byte = value[*valuePos];
byte = buffer[*bufferPos];
// Shift the lower order 7 encoded bits into the uint64 in reverse order
result |= (uint64_t)(byte & 0x7f) << (7 * valueIdx);
result |= (uint64_t)(byte & 0x7f) << (7 * bufferIdx);
// Increment value position to indicate that the byte has been processed
(*valuePos)++;
// Increment buffer position to indicate that the byte has been processed
(*bufferPos)++;
// Done if the high order bit is not set to indicate more data
if (byte < 0x80)
@ -634,7 +639,7 @@ cvtUInt64FromVarInt128(const uint8_t *const value, size_t *const valuePos)
}
// By this point all bytes should have been read so error if this is not the case. This could be due to a coding error or
// corrupton in the data stream.
// corruption in the data stream.
if (byte >= 0x80)
THROW(FormatError, "unterminated varint-128 integer");

2
src/common/type/convert.h

@ -117,7 +117,7 @@ cvtZSubNToUInt64(const char *const value, const size_t offset, const size_t size
// Convert uint64 to base-128 varint and vice versa
void cvtUInt64ToVarInt128(uint64_t value, uint8_t *buffer, size_t *bufferPos, size_t bufferSize);
uint64_t cvtUInt64FromVarInt128(const uint8_t *value, size_t *valuePos);
uint64_t cvtUInt64FromVarInt128(const uint8_t *buffer, size_t *bufferPos, size_t bufferSize);
// Convert boolean to zero-terminated string. Use cvtBoolToConstZ() whenever possible since it is more efficient.
size_t cvtBoolToZ(bool value, char *buffer, size_t bufferSize);

20
src/info/manifest.c

@ -246,14 +246,14 @@ manifestFileUnpack(const Manifest *const manifest, const ManifestFilePack *const
bufferPos += sizeof(StringPub) + strSize(result.name) + 1;
// Flags
const uint64_t flag = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos);
const uint64_t flag = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
// Size
result.size = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos);
result.size = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
// Timestamp
result.timestamp =
manifestPackBaseTime - (time_t)cvtInt64FromZigZag(cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos));
manifestPackBaseTime - (time_t)cvtInt64FromZigZag(cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX));
// Checksum page
result.checksumPage = flag & (1 << manifestFilePackFlagChecksumPage) ? true : false;
@ -264,33 +264,33 @@ manifestFileUnpack(const Manifest *const manifest, const ManifestFilePack *const
// Reference
if (flag & (1 << manifestFilePackFlagReference))
result.reference = (const String *)(uintptr_t)cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos);
result.reference = (const String *)(uintptr_t)cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
// Mode
if (flag & (1 << manifestFilePackFlagMode))
result.mode = (mode_t)cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos);
result.mode = (mode_t)cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
else
result.mode = manifest->fileModeDefault;
// User/group
if (flag & (1 << manifestFilePackFlagUser))
result.user = (const String *)(uintptr_t)cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos);
result.user = (const String *)(uintptr_t)cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
else if (!(flag & (1 << manifestFilePackFlagUserNull)))
result.user = manifest->fileUserDefault;
if (flag & (1 << manifestFilePackFlagGroup))
result.group = (const String *)(uintptr_t)cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos);
result.group = (const String *)(uintptr_t)cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
else if (!(flag & (1 << manifestFilePackFlagGroupNull)))
result.group = manifest->fileGroupDefault;
// Repo size
result.sizeRepo = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos);
result.sizeRepo = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
// Bundle
if (flag & (1 << manifestFilePackFlagBundle))
{
result.bundleId = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos);
result.bundleOffset = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos);
result.bundleId = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
result.bundleOffset = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
}
// Checksum page error

9
test/src/module/common/typeConvertTest.c

@ -224,7 +224,10 @@ testRun(void)
TEST_RESULT_UINT(bufferPos, 10, "check buffer position");
bufferPos = 0;
TEST_RESULT_UINT(cvtUInt64FromVarInt128(buffer, &bufferPos), 0xFFFFFFFFFFFFFFFF, "to uint64");
TEST_ERROR(cvtUInt64FromVarInt128(buffer, &bufferPos, 1), FormatError, "buffer position is beyond buffer size");
bufferPos = 0;
TEST_RESULT_UINT(cvtUInt64FromVarInt128(buffer, &bufferPos, sizeof(buffer)), 0xFFFFFFFFFFFFFFFF, "to uint64");
TEST_RESULT_UINT(bufferPos, 10, "check buffer position");
bufferPos = 0;
@ -232,13 +235,13 @@ testRun(void)
TEST_RESULT_UINT(bufferPos, 1, "check buffer position");
bufferPos = 0;
TEST_RESULT_UINT(cvtUInt64FromVarInt128(buffer, &bufferPos), 0, "to uint64");
TEST_RESULT_UINT(cvtUInt64FromVarInt128(buffer, &bufferPos, sizeof(buffer)), 0, "to uint64");
TEST_RESULT_UINT(bufferPos, 1, "check buffer position");
uint8_t buffer2[CVT_VARINT128_BUFFER_SIZE + 1];
memset(buffer2, 0xFF, sizeof(buffer2));
bufferPos = 0;
TEST_ERROR(cvtUInt64FromVarInt128(buffer2, &bufferPos), FormatError, "unterminated varint-128 integer");
TEST_ERROR(cvtUInt64FromVarInt128(buffer2, &bufferPos, sizeof(buffer)), FormatError, "unterminated varint-128 integer");
}
FUNCTION_HARNESS_RETURN_VOID();