make edge case handing in AsyncCallbackJsonWebHandler more robust

Author: ayavilevich

src/AsyncJson.cpp

@@ -117,20 +117,35 @@ void AsyncCallbackJsonWebHandler::handleRequest(AsyncWebServerRequest *request)
       JsonVariant json;
       _onRequest(request, json);
       return;
-    } else if (request->_tempObject != NULL) {
+    }
+    // this is not a GET
+    // check if body is too large, if it is, don't parse
+    if (request->contentLength() > _maxContentLength)
+    {
+      request->send(413);
+      return;
+    }
 
+    // try to parse body as JSON
+    if (request->_tempObject != NULL)
+    {
+      size_t dataSize = min(request->contentLength(), request->_tempSize); // smaller value of contentLength or the size of the buffer. normally those should match.
 #if ARDUINOJSON_VERSION_MAJOR == 5
       DynamicJsonBuffer jsonBuffer;
-      JsonVariant json = jsonBuffer.parse((uint8_t *)(request->_tempObject));
+      uint8_t * p = (uint8_t *)(request->_tempObject);
+      p[dataSize] = '\0'; // null terminate, assume we allocated one extra char
+      // parse can only get null terminated strings as parameters
+      JsonVariant json = jsonBuffer.parse(p);
       if (json.success()) {
 #elif ARDUINOJSON_VERSION_MAJOR == 6
       DynamicJsonDocument jsonBuffer(this->maxJsonBufferSize);
-      DeserializationError error = deserializeJson(jsonBuffer, (uint8_t *)(request->_tempObject), request->contentLength());
+      DeserializationError error = deserializeJson(jsonBuffer, (uint8_t *)(request->_tempObject), dataSize);
       if (!error) {
         JsonVariant json = jsonBuffer.as<JsonVariant>();
 #else
       JsonDocument jsonBuffer;
-      DeserializationError error = deserializeJson(jsonBuffer, (uint8_t *)(request->_tempObject), request->contentLength());
+      // deserializeJson expects a null terminated string or a pointer plus length
+      DeserializationError error = deserializeJson(jsonBuffer, (uint8_t *)(request->_tempObject), dataSize);
       if (!error) {
         JsonVariant json = jsonBuffer.as<JsonVariant>();
 #endif
@@ -139,27 +154,40 @@ void AsyncCallbackJsonWebHandler::handleRequest(AsyncWebServerRequest *request)
         return;
       }
     }
-    request->send(_contentLength > _maxContentLength ? 413 : 400);
-  } else {
+    // there is no body, no buffer or we had an error parsing the body
+    request->send(400);
+  } else { // if no _onRequest
     request->send(500);
   }
 }
 
 void AsyncCallbackJsonWebHandler::handleBody(AsyncWebServerRequest *request, uint8_t *data, size_t len, size_t index, size_t total) {
   if (_onRequest) {
-    _contentLength = total;
-    if (total > 0 && request->_tempObject == NULL && total < _maxContentLength) {
-      request->_tempObject = malloc(total);
-      if (request->_tempObject == NULL) {
+    if (total > 0 && request->_tempObject == NULL && total < _maxContentLength) { // if request content length is valid size and we have no content buffer yet
+      request->_tempObject = malloc(total + 1); // allocate one additional byte so we can null terminate this buffer (needed for ArduinoJson 5)
+      if (request->_tempObject == NULL) { // if allocation failed
 #ifdef ESP32
         log_e("Failed to allocate");
 #endif
         request->abort();
         return;
       }
+      request->_tempSize = total; // store the size of allocation we made into _tempSize
     }
     if (request->_tempObject != NULL) {
-      memcpy((uint8_t *)(request->_tempObject) + index, data, len);
+      // check if the buffer is the right size so we don't write out of bounds
+      if (request->_tempSize >= total)
+      {
+        memcpy((uint8_t *)(request->_tempObject) + index, data, len);
+      }
+      else
+      {
+#ifdef ESP32
+        log_e("Bad size of temp buffer");
+#endif
+        request->abort();
+        return;
+      }
     }
   }
 }

src/AsyncJson.h

@@ -79,7 +79,6 @@ class AsyncCallbackJsonWebHandler : public AsyncWebHandler {
   String _uri;
   WebRequestMethodComposite _method;
   ArJsonRequestHandlerFunction _onRequest;
-  size_t _contentLength;
 #if ARDUINOJSON_VERSION_MAJOR == 6
   size_t maxJsonBufferSize;
 #endif

src/ESPAsyncWebServer.h

@@ -261,6 +261,7 @@ class AsyncWebServerRequest {
 public:
   File _tempFile;
   void *_tempObject;
+  size_t _tempSize;
 
   AsyncWebServerRequest(AsyncWebServer *, AsyncClient *);
   ~AsyncWebServerRequest();

src/WebRequest.cpp

@@ -25,7 +25,7 @@ AsyncWebServerRequest::AsyncWebServerRequest(AsyncWebServer *s, AsyncClient *c)
   : _client(c), _server(s), _handler(NULL), _response(NULL), _temp(), _parseState(PARSE_REQ_START), _version(0), _method(HTTP_ANY), _url(), _host(),
     _contentType(), _boundary(), _authorization(), _reqconntype(RCT_HTTP), _authMethod(AsyncAuthType::AUTH_NONE), _isMultipart(false), _isPlainPost(false),
     _expectingContinue(false), _contentLength(0), _parsedLength(0), _multiParseState(0), _boundaryPosition(0), _itemStartIndex(0), _itemSize(0), _itemName(),
-    _itemFilename(), _itemType(), _itemValue(), _itemBuffer(0), _itemBufferIndex(0), _itemIsFile(false), _tempObject(NULL) {
+    _itemFilename(), _itemType(), _itemValue(), _itemBuffer(0), _itemBufferIndex(0), _itemIsFile(false), _tempObject(NULL), _tempSize(0) {
   c->onError(
     [](void *r, AsyncClient *c, int8_t error) {
       (void)c;