Commit 6393cf96 authored by Sean McBride's avatar Sean McBride Committed by Kitware Robot
Browse files

Merge topic 'base64-overrun'

35b29336

 Fixed buffer overrun by rewriting Decode() method to be safer
Acked-by: Kitware Robot's avatarKitware Robot <kwrobot@kitware.com>
Acked-by: Ben Boeckel's avatarBen Boeckel <ben.boeckel@kitware.com>
Merge-request: !1124
parents 89a6cb5f 35b29336
......@@ -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