From ac7d9af48fa9199dcbc4433b2a56b2b159593680 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Thu, 12 Jan 2017 13:22:19 +0100 Subject: [PATCH] attempt to validate utf8 sequences in vst --- arangod/GeneralServer/VppCommTask.cpp | 12 +++---- arangod/GeneralServer/VppNetwork.h | 35 +++++++++---------- .../RestHandler/RestVocbaseBaseHandler.cpp | 1 - arangod/V8Server/v8-actions.cpp | 4 --- lib/Rest/HttpRequest.cpp | 6 ++-- lib/Rest/VppRequest.cpp | 3 +- 6 files changed, 27 insertions(+), 34 deletions(-) diff --git a/arangod/GeneralServer/VppCommTask.cpp b/arangod/GeneralServer/VppCommTask.cpp index 28a81aaf18..34720e0c9a 100644 --- a/arangod/GeneralServer/VppCommTask.cpp +++ b/arangod/GeneralServer/VppCommTask.cpp @@ -27,7 +27,6 @@ #include #include -#include #include #include @@ -49,9 +48,6 @@ #include "Utils/Events.h" #include "VocBase/ticks.h" -#include -#include - using namespace arangodb; using namespace arangodb::basics; using namespace arangodb::rest; @@ -292,7 +288,7 @@ bool VppCommTask::processRead(double startTime) { handleSimpleError(rest::ResponseCode::BAD, chunkHeader._messageID); LOG_TOPIC(DEBUG, Logger::COMMUNICATION) << "VppCommTask: " - << std::string("VPack Validation failed!") + e.what(); + << "VPack Validation failed: " << e.what(); closeTask(rest::ResponseCode::BAD); return false; } @@ -434,14 +430,14 @@ boost::optional VppCommTask::getMessageFromSingleChunk( TRI_ERROR_ARANGO_DATABASE_NOT_FOUND, e.what(), chunkHeader._messageID); LOG_TOPIC(DEBUG, Logger::COMMUNICATION) << "VppCommTask: " - << "VPack Validation failed!" + << "VPack Validation failed: " << e.what(); closeTask(rest::ResponseCode::BAD); return false; } catch (...) { handleSimpleError(rest::ResponseCode::BAD, chunkHeader._messageID); LOG_TOPIC(DEBUG, Logger::COMMUNICATION) << "VppCommTask: " - << "VPack Validation failed!"; + << "VPack Validation failed"; closeTask(rest::ResponseCode::BAD); return false; } @@ -523,7 +519,7 @@ boost::optional VppCommTask::getMessageFromMultiChunks( TRI_ERROR_ARANGO_DATABASE_NOT_FOUND, e.what(), chunkHeader._messageID); LOG_TOPIC(DEBUG, Logger::COMMUNICATION) << "VppCommTask: " - << "VPack Validation failed!" + << "VPack Validation failed: " << e.what(); closeTask(rest::ResponseCode::BAD); return false; diff --git a/arangod/GeneralServer/VppNetwork.h b/arangod/GeneralServer/VppNetwork.h index 22eeda80b5..4dd6d3a51a 100644 --- a/arangod/GeneralServer/VppNetwork.h +++ b/arangod/GeneralServer/VppNetwork.h @@ -28,6 +28,7 @@ #include "Basics/VelocyPackHelper.h" #include "Logger/LoggerFeature.h" +#include #include #include #include @@ -37,32 +38,30 @@ using namespace arangodb; -inline std::size_t validateAndCount(char const* vpHeaderStart, +inline std::size_t validateAndCount(char const* vpStart, char const* vpEnd) { + VPackOptions validationOptions = VPackOptions::Defaults; + validationOptions.validateUtf8Strings = true; + VPackValidator validator(&validationOptions); + + std::size_t numPayloads = 0; + try { - VPackValidator validator; // check for slice start to the end of Chunk // isSubPart allows the slice to be shorter than the checked buffer. - validator.validate(vpHeaderStart, std::distance(vpHeaderStart, vpEnd), - /*isSubPart =*/true); - - VPackSlice vpHeader(vpHeaderStart); - auto vpPayloadStart = vpHeaderStart + vpHeader.byteSize(); - - std::size_t numPayloads = 0; - while (vpPayloadStart != vpEnd) { - // validate - validator.validate(vpPayloadStart, std::distance(vpPayloadStart, vpEnd), - true); + do { + validator.validate(vpStart, std::distance(vpStart, vpEnd), + /*isSubPart =*/true); + // get offset to next - VPackSlice tmp(vpPayloadStart); - vpPayloadStart += tmp.byteSize(); + VPackSlice tmp(vpStart); + vpStart += tmp.byteSize(); numPayloads++; - } - return numPayloads; + } while (vpStart != vpEnd); + return numPayloads - 1; } catch (std::exception const& e) { throw std::runtime_error( - std::string("error during validation of incoming VPack") + e.what()); + std::string("error during validation of incoming VPack: ") + e.what()); } } diff --git a/arangod/RestHandler/RestVocbaseBaseHandler.cpp b/arangod/RestHandler/RestVocbaseBaseHandler.cpp index 5f7cc9e2ee..6dd0829207 100644 --- a/arangod/RestHandler/RestVocbaseBaseHandler.cpp +++ b/arangod/RestHandler/RestVocbaseBaseHandler.cpp @@ -40,7 +40,6 @@ #include #include #include -#include #include using namespace arangodb; diff --git a/arangod/V8Server/v8-actions.cpp b/arangod/V8Server/v8-actions.cpp index 5701d6e19c..c75935e6a0 100644 --- a/arangod/V8Server/v8-actions.cpp +++ b/arangod/V8Server/v8-actions.cpp @@ -414,12 +414,8 @@ static v8::Handle RequestCppToV8(v8::Isolate* isolate, headers["content-length"] = StringUtils::itoa(request->contentLength()); } else if (rest::ContentType::VPACK == request->contentType()) { // the VPACK is passed as it is to to Javascript - // should we convert and validate here in a central place? - // should the work be done in javascript // FIXME not every VPack can be converted to JSON VPackSlice slice = request->payload(); - VPackValidator validator; - validator.validate(slice.start(), slice.byteSize()); std::string jsonString = slice.toJson(); LOG_TOPIC(DEBUG, Logger::COMMUNICATION) diff --git a/lib/Rest/HttpRequest.cpp b/lib/Rest/HttpRequest.cpp index 2bc628a164..f560d87be2 100644 --- a/lib/Rest/HttpRequest.cpp +++ b/lib/Rest/HttpRequest.cpp @@ -739,7 +739,7 @@ void HttpRequest::setBody(char const* body, size_t length) { } VPackSlice HttpRequest::payload(VPackOptions const* options) { - // check options for nullptr? + TRI_ASSERT(options != nullptr); if (_contentType == ContentType::JSON) { if (!_body.empty()) { @@ -752,7 +752,9 @@ VPackSlice HttpRequest::payload(VPackOptions const* options) { } return VPackSlice::noneSlice(); // no body } else /*VPACK*/ { - VPackValidator validator; + VPackOptions validationOptions = *options; // intentional copy + validationOptions.validateUtf8Strings = true; + VPackValidator validator(&validationOptions); validator.validate(_body.c_str(), _body.length()); return VPackSlice(_body.c_str()); } diff --git a/lib/Rest/VppRequest.cpp b/lib/Rest/VppRequest.cpp index f1225eeb93..ef365b6690 100644 --- a/lib/Rest/VppRequest.cpp +++ b/lib/Rest/VppRequest.cpp @@ -28,7 +28,6 @@ #include #include #include -#include #include #include "Basics/StaticStrings.h" @@ -74,6 +73,8 @@ VppRequest::VppRequest(ConnectionInfo const& connectionInfo, } VPackSlice VppRequest::payload(VPackOptions const* options) { + // message does not need to be validated here, as it was already + // validated before return _message.payload(); }