From 1c4163b010cc95791863e8f07bf4154e2ca7b011 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Tue, 5 Nov 2013 11:12:14 -0800 Subject: [PATCH 1/4] add guards against buffer overflow in voxel server for edit/delete voxel packets --- .../src/VoxelServerPacketProcessor.cpp | 32 +++++++++++-------- libraries/voxels/src/VoxelTree.cpp | 14 +++++--- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/libraries/voxel-server-library/src/VoxelServerPacketProcessor.cpp b/libraries/voxel-server-library/src/VoxelServerPacketProcessor.cpp index 9b79ae121a..a60f8e89ee 100644 --- a/libraries/voxel-server-library/src/VoxelServerPacketProcessor.cpp +++ b/libraries/voxel-server-library/src/VoxelServerPacketProcessor.cpp @@ -49,25 +49,31 @@ void VoxelServerPacketProcessor::processPacket(sockaddr& senderAddress, unsigned int atByte = numBytesPacketHeader + sizeof(itemNumber); unsigned char* voxelData = (unsigned char*)&packetData[atByte]; while (atByte < packetLength) { - unsigned char octets = (unsigned char)*voxelData; + unsigned char octets = numberOfThreeBitSectionsInCode(voxelData); const int COLOR_SIZE_IN_BYTES = 3; int voxelDataSize = bytesRequiredForCodeLength(octets) + COLOR_SIZE_IN_BYTES; int voxelCodeSize = bytesRequiredForCodeLength(octets); - if (_myServer->wantShowAnimationDebug()) { - int red = voxelData[voxelCodeSize + 0]; - int green = voxelData[voxelCodeSize + 1]; - int blue = voxelData[voxelCodeSize + 2]; + if (atByte + voxelDataSize <= packetLength) { + if (_myServer->wantShowAnimationDebug()) { + int red = voxelData[voxelCodeSize + 0]; + int green = voxelData[voxelCodeSize + 1]; + int blue = voxelData[voxelCodeSize + 2]; - float* vertices = firstVertexForCode(voxelData); - printf("inserting voxel: %f,%f,%f r=%d,g=%d,b=%d\n", vertices[0], vertices[1], vertices[2], red, green, blue); - delete[] vertices; - } + float* vertices = firstVertexForCode(voxelData); + printf("inserting voxel: %f,%f,%f r=%d,g=%d,b=%d\n", vertices[0], vertices[1], vertices[2], red, green, blue); + delete[] vertices; + } - _myServer->getServerTree().readCodeColorBufferToTree(voxelData, destructive); - // skip to next - voxelData += voxelDataSize; - atByte += voxelDataSize; + _myServer->getServerTree().readCodeColorBufferToTree(voxelData, destructive); + + // skip to next voxel edit record in the packet + voxelData += voxelDataSize; + atByte += voxelDataSize; + } else { + printf("WARNING! Got voxel edit record that would overflow buffer, bailing processing of packet!\n"); + break; + } } // Make sure our Node and NodeList knows we've heard from this node. diff --git a/libraries/voxels/src/VoxelTree.cpp b/libraries/voxels/src/VoxelTree.cpp index 251a6300f2..ec86912a51 100644 --- a/libraries/voxels/src/VoxelTree.cpp +++ b/libraries/voxels/src/VoxelTree.cpp @@ -583,11 +583,15 @@ void VoxelTree::processRemoveVoxelBitstream(unsigned char * bitstream, int buffe while (atByte < bufferSizeBytes) { int codeLength = numberOfThreeBitSectionsInCode(voxelCode); int voxelDataSize = bytesRequiredForCodeLength(codeLength) + SIZE_OF_COLOR_DATA; - - deleteVoxelCodeFromTree(voxelCode, COLLAPSE_EMPTY_TREE); - - voxelCode+=voxelDataSize; - atByte+=voxelDataSize; + + if (atByte + voxelDataSize <= bufferSizeBytes) { + deleteVoxelCodeFromTree(voxelCode, COLLAPSE_EMPTY_TREE); + voxelCode+=voxelDataSize; + atByte+=voxelDataSize; + } else { + printf("WARNING! Got remove voxel bitstream that would overflow buffer, bailing processing!\n"); + break; + } } } From adf0feed296b2ebb7e007e2907dd720203a01f52 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Tue, 5 Nov 2013 11:14:29 -0800 Subject: [PATCH 2/4] style clenaup --- .../voxel-server-library/src/VoxelServerPacketProcessor.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/voxel-server-library/src/VoxelServerPacketProcessor.cpp b/libraries/voxel-server-library/src/VoxelServerPacketProcessor.cpp index a60f8e89ee..53a6267a7a 100644 --- a/libraries/voxel-server-library/src/VoxelServerPacketProcessor.cpp +++ b/libraries/voxel-server-library/src/VoxelServerPacketProcessor.cpp @@ -56,9 +56,9 @@ void VoxelServerPacketProcessor::processPacket(sockaddr& senderAddress, unsigned if (atByte + voxelDataSize <= packetLength) { if (_myServer->wantShowAnimationDebug()) { - int red = voxelData[voxelCodeSize + 0]; - int green = voxelData[voxelCodeSize + 1]; - int blue = voxelData[voxelCodeSize + 2]; + int red = voxelData[voxelCodeSize + RED_INDEX]; + int green = voxelData[voxelCodeSize + GREEN_INDEX]; + int blue = voxelData[voxelCodeSize + BLUE_INDEX]; float* vertices = firstVertexForCode(voxelData); printf("inserting voxel: %f,%f,%f r=%d,g=%d,b=%d\n", vertices[0], vertices[1], vertices[2], red, green, blue); From 4cab991c02e1e67a6b97098a935c1dbbd15c4ded Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Tue, 5 Nov 2013 11:24:51 -0800 Subject: [PATCH 3/4] style cleanup --- libraries/voxels/src/VoxelTree.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/voxels/src/VoxelTree.cpp b/libraries/voxels/src/VoxelTree.cpp index ec86912a51..e5232718e6 100644 --- a/libraries/voxels/src/VoxelTree.cpp +++ b/libraries/voxels/src/VoxelTree.cpp @@ -586,8 +586,8 @@ void VoxelTree::processRemoveVoxelBitstream(unsigned char * bitstream, int buffe if (atByte + voxelDataSize <= bufferSizeBytes) { deleteVoxelCodeFromTree(voxelCode, COLLAPSE_EMPTY_TREE); - voxelCode+=voxelDataSize; - atByte+=voxelDataSize; + voxelCode += voxelDataSize; + atByte + =voxelDataSize; } else { printf("WARNING! Got remove voxel bitstream that would overflow buffer, bailing processing!\n"); break; From 9060020279ec119c238744a465733526ceb7ac60 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Tue, 5 Nov 2013 11:33:28 -0800 Subject: [PATCH 4/4] style cleanup --- libraries/voxels/src/VoxelTree.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/voxels/src/VoxelTree.cpp b/libraries/voxels/src/VoxelTree.cpp index e5232718e6..f7f50788c5 100644 --- a/libraries/voxels/src/VoxelTree.cpp +++ b/libraries/voxels/src/VoxelTree.cpp @@ -587,7 +587,7 @@ void VoxelTree::processRemoveVoxelBitstream(unsigned char * bitstream, int buffe if (atByte + voxelDataSize <= bufferSizeBytes) { deleteVoxelCodeFromTree(voxelCode, COLLAPSE_EMPTY_TREE); voxelCode += voxelDataSize; - atByte + =voxelDataSize; + atByte += voxelDataSize; } else { printf("WARNING! Got remove voxel bitstream that would overflow buffer, bailing processing!\n"); break;