ESP8266WebServer - fix possible memory leak in request argument handl…

…ing (#9076) * fix possible leak of _postArgs array in case of returning early from _parseForm(). * don't use _postArgs member, but instead use a new local variable postArgs instead. * same for _postArgsLen member vs.local postArgsLen. * remove useless NULL pointer check before delete(). * Remove _postArgs member from ESP8266WebServer.h * Remove searching through always empty _postArgs array in ESP8266WebServer-impl.h
esp8266 · Feb 9, 2024 · 16e1918 · 16e1918
1 parent de1029f
commit 16e1918
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 27 deletions.
diff --git a/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h b/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h
@@ -590,10 +590,6 @@ const String& ESP8266WebServerTemplate<ServerType>::pathArg(unsigned int i) cons
 
 template <typename ServerType>
 const String& ESP8266WebServerTemplate<ServerType>::arg(const String& name) const {
-  for (int j = 0; j < _postArgsLen; ++j) {
-    if ( _postArgs[j].key == name )
-      return _postArgs[j].value;
-  }
   for (int i = 0; i < _currentArgCount + _currentArgsHavePlain; ++i) {
     if ( _currentArgs[i].key == name )
       return _currentArgs[i].value;
@@ -622,10 +618,6 @@ int ESP8266WebServerTemplate<ServerType>::args() const {
 
 template <typename ServerType>
 bool ESP8266WebServerTemplate<ServerType>::hasArg(const String& name) const {
-  for (int j = 0; j < _postArgsLen; ++j) {
-    if (_postArgs[j].key == name)
-      return true;
-  }
   for (int i = 0; i < _currentArgCount + _currentArgsHavePlain; ++i) {
     if (_currentArgs[i].key == name)
       return true;

diff --git a/libraries/ESP8266WebServer/src/ESP8266WebServer.h b/libraries/ESP8266WebServer/src/ESP8266WebServer.h
@@ -323,8 +323,6 @@ class ESP8266WebServerTemplate
   RequestArgument* _currentArgs = nullptr;
   int              _currentArgsHavePlain = 0;
   std::unique_ptr<HTTPUpload> _currentUpload;
-  int              _postArgsLen = 0;
-  RequestArgument* _postArgs = nullptr;
 
   int              _headerKeysCount = 0;
   RequestArgument* _currentHeaders = nullptr;
@@ -352,4 +350,4 @@ class ESP8266WebServerTemplate
 using ESP8266WebServer = esp8266webserver::ESP8266WebServerTemplate<WiFiServer>;
 using RequestHandler = esp8266webserver::RequestHandler<WiFiServer>;
 
-#endif //ESP8266WEBSERVER_H
+#endif //ESP8266WEBSERVER_H
diff --git a/libraries/ESP8266WebServer/src/Parsing-impl.h b/libraries/ESP8266WebServer/src/Parsing-impl.h
@@ -358,9 +358,8 @@ bool ESP8266WebServerTemplate<ServerType>::_parseForm(ClientType& client, const
   client.readStringUntil('\n');
   //start reading the form
   if (line == ("--"+boundary)){
-    if(_postArgs) delete[] _postArgs;
-    _postArgs = new RequestArgument[WEBSERVER_MAX_POST_ARGS];
-    _postArgsLen = 0;
+    std::unique_ptr<RequestArgument[]> postArgs(new RequestArgument[WEBSERVER_MAX_POST_ARGS]);
+    int postArgsLen = 0;
     while(1){
       String argName;
       String argValue;
@@ -408,7 +407,7 @@ bool ESP8266WebServerTemplate<ServerType>::_parseForm(ClientType& client, const
             }
             DBGWS("PostArg Value: %s\n\n", argValue.c_str());
 
-            RequestArgument& arg = _postArgs[_postArgsLen++];
+            RequestArgument& arg = postArgs[postArgsLen++];
             arg.key = argName;
             arg.value = argValue;
 
@@ -488,25 +487,20 @@ bool ESP8266WebServerTemplate<ServerType>::_parseForm(ClientType& client, const
     }
 
     int iarg;
-    int totalArgs = ((WEBSERVER_MAX_POST_ARGS - _postArgsLen) < _currentArgCount)?(WEBSERVER_MAX_POST_ARGS - _postArgsLen):_currentArgCount;
+    int totalArgs = ((WEBSERVER_MAX_POST_ARGS - postArgsLen) < _currentArgCount)?(WEBSERVER_MAX_POST_ARGS - postArgsLen):_currentArgCount;
     for (iarg = 0; iarg < totalArgs; iarg++){
-      RequestArgument& arg = _postArgs[_postArgsLen++];
+      RequestArgument& arg = postArgs[postArgsLen++];
       arg.key = _currentArgs[iarg].key;
       arg.value = _currentArgs[iarg].value;
     }
-    if (_currentArgs) delete[] _currentArgs;
-    _currentArgs = new RequestArgument[_postArgsLen];
-    for (iarg = 0; iarg < _postArgsLen; iarg++){
+    delete[] _currentArgs;
+    _currentArgs = new RequestArgument[postArgsLen];
+    for (iarg = 0; iarg < postArgsLen; iarg++){
       RequestArgument& arg = _currentArgs[iarg];
-      arg.key = _postArgs[iarg].key;
-      arg.value = _postArgs[iarg].value;
+      arg.key = postArgs[iarg].key;
+      arg.value = postArgs[iarg].value;
     }
     _currentArgCount = iarg;
-    if (_postArgs) {
-      delete[] _postArgs;
-      _postArgs = nullptr;
-      _postArgsLen = 0;
-    }
     return true;
   }
   DBGWS("Error: line: %s\n", line.c_str());