Commit 35b29336 authored by Sean McBride's avatar Sean McBride
Browse files

Fixed buffer overrun by rewriting Decode() method to be safer

Created new DecodeSafely() method that takes buffer lengths,
which means we can be sure not to overrun any buffer.

Under ASan, we see that the vtkWebCorePython-TestDataEncoder test
does indeed overrun a buffer with the old method.

Deprecated the old method.
parent 8453c6ce
......@@ -210,12 +210,15 @@ int vtkBase64Utilities::DecodeTriplet(unsigned char i0,
return 3;
}
#if !defined(VTK_LEGACY_REMOVE)
//----------------------------------------------------------------------------
unsigned long vtkBase64Utilities::Decode(const unsigned char *input,
unsigned long length,
unsigned char *output,
unsigned long max_input_length)
{
VTK_LEGACY_REPLACED_BODY(Decode, "VTK 7.1", DecodeSafely);
const unsigned char *ptr = input;
unsigned char *optr = output;
......@@ -275,3 +278,58 @@ unsigned long vtkBase64Utilities::Decode(const unsigned char *input,
return optr - output;
}
#endif
//----------------------------------------------------------------------------
size_t vtkBase64Utilities::DecodeSafely(const unsigned char *input,
size_t inputLen,
unsigned char *output,
size_t outputLen)
{
assert(input);
assert(output);
// Nonsense small input or no space for any output
if ((inputLen < 4) || (outputLen == 0))
{
return 0;
}
// Consume 4 ASCII chars of input at a time, until less than 4 left
size_t inIdx = 0, outIdx = 0;
while (inIdx <= inputLen-4)
{
// Decode 4 ASCII characters into 0, 1, 2, or 3 bytes
unsigned char o0, o1, o2;
int bytesDecoded = vtkBase64Utilities::DecodeTriplet(
input[inIdx+0], input[inIdx+1], input[inIdx+2], input[inIdx+3],
&o0, &o1, &o2);
assert((bytesDecoded >= 0) && (bytesDecoded <= 3));
if ((bytesDecoded >= 1) && (outIdx < outputLen))
{
output[outIdx++] = o0;
}
if ((bytesDecoded >= 2) && (outIdx < outputLen))
{
output[outIdx++] = o1;
}
if ((bytesDecoded >= 3) && (outIdx < outputLen))
{
output[outIdx++] = o2;
}
// If fewer than 3 bytes resulted from decoding (in this pass),
// then the input stream has nothing else decodable, so end.
if (bytesDecoded < 3)
{
return outIdx;
}
// Consumed a whole 4 of input and got 3 bytes of output, continue
inIdx += 4;
assert(bytesDecoded == 3);
}
return outIdx;
}
......@@ -60,7 +60,7 @@ public:
// encoded stream into the output buffer. Return the length of
// the encoded stream. Note that the output buffer must be allocated
// by the caller (length * 1.5 should be a safe estimate).
// If 'mark_end' is true than an extra set of 4 bytes is added
// If 'mark_end' is true then an extra set of 4 bytes is added
// to the end of the stream if the input is a multiple of 3 bytes.
// These bytes are invalid chars and therefore they will stop the decoder
// thus enabling the caller to decode a stream without actually knowing
......@@ -75,6 +75,7 @@ public:
// Description:
// Decode 4 bytes into 3 bytes.
// Return the number of bytes actually decoded (0 to 3, inclusive).
static int DecodeTriplet(unsigned char i0,
unsigned char i1,
unsigned char i2,
......@@ -88,15 +89,29 @@ public:
// into the output buffer until 'length' bytes have been decoded.
// Return the real length of the decoded stream (which should be equal to
// 'length'). Note that the output buffer must be allocated by the caller.
// If 'max_input_length' is not null, then it specifies the number of
// If 'max_input_length' is not 0, then it specifies the number of
// encoded bytes that should be at most read from the input buffer. In
// that case the 'length' parameter is ignored. This enables the caller
// to decode a stream without actually knowing how much decoded data to
// expect (of course, the buffer must be large enough).
static unsigned long Decode(const unsigned char *input,
unsigned long length,
unsigned char *output,
unsigned long max_input_length = 0);
// \deprecated: This method can easily overrun its buffers, use DecodeSafely.
VTK_LEGACY(static unsigned long Decode(const unsigned char *input,
unsigned long length,
unsigned char *output,
unsigned long max_input_length = 0));
// Description:
// Decode 4 bytes at a time from the input buffer and store the decoded
// stream into the output buffer. The required output buffer size must be
// determined and allocated by the caller. The needed output space is
// always less than the input buffer size, so a good first order
// approximation is to allocate the same size. Base64 encoding is about
// 4/3 overhead, so a tighter bound is possible.
// Return the number of bytes atually placed into the output buffer.
static size_t DecodeSafely(const unsigned char *input,
size_t inputLen,
unsigned char *output,
size_t outputLen);
protected:
vtkBase64Utilities() {}
......
......@@ -779,10 +779,11 @@ vtkImageData *vtkGetNoiseResource()
unsigned char* binaryInput
= new unsigned char[file_noise200x200_vtk_decoded_length + 10];
unsigned long binarylength = vtkBase64Utilities::Decode(
unsigned long binarylength = vtkBase64Utilities::DecodeSafely(
reinterpret_cast<const unsigned char*>(base64string.c_str()),
static_cast<unsigned long>(base64string.length()),
binaryInput);
base64string.length(),
binaryInput,
file_noise200x200_vtk_decoded_length + 10);
assert("check valid_length"
&& (binarylength == file_noise200x200_vtk_decoded_length));
......
......@@ -774,10 +774,11 @@ vtkImageData *vtkGetNoiseResource()
unsigned char* binaryInput
= new unsigned char[file_noise200x200_vtk_decoded_length + 10];
unsigned long binarylength = vtkBase64Utilities::Decode(
unsigned long binarylength = vtkBase64Utilities::DecodeSafely(
reinterpret_cast<const unsigned char*>(base64string.c_str()),
static_cast<unsigned long>(base64string.length()),
binaryInput);
base64string.length(),
binaryInput,
file_noise200x200_vtk_decoded_length + 10);
assert("check valid_length"
&& (binarylength == file_noise200x200_vtk_decoded_length));
......
......@@ -40,10 +40,10 @@ class TestDataEncoder(Testing.vtkTest):
base64String = encoder.EncodeAsBase64Png(imgData).encode('ascii')
# Now Base64 decode the string back to PNG image data bytes
outputBuffer = bytearray(120000)
inputArray = array.array('B', base64String)
outputBuffer = bytearray(len(inputArray))
utils = vtk.vtkIOCore.vtkBase64Utilities()
actualLength = utils.Decode(inputArray, 120000, outputBuffer)
actualLength = utils.DecodeSafely(inputArray, len(inputArray), outputBuffer, len(outputBuffer))
outputArray = bytearray(actualLength)
outputArray[:] = outputBuffer[0:actualLength]
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment