HttpClient buffer too small #38

Activity

1. ScruffR commented on Dec 12, 2019

   

   ScruffR

   on Dec 12, 2019

   Last edited by ScruffR

   More actions

   Additionally to that I do see a potential buffer overflow issue with this code

           while (client.available()) {
               ...
               if (bufferPosition < sizeof(buffer)-1) {
                   buffer[bufferPosition] = c;
               } else if ((bufferPosition == sizeof(buffer)-1)) {
                   buffer[bufferPosition] = '\0'; // Null-terminate buffer
                   client.stop();
                   error = true;
   
                   #ifdef LOGGING
                   Serial.println("HttpClient>\tError: Response body larger than buffer.");
                   #endif
               }
               bufferPosition++;
           }
           buffer[bufferPosition] = '\0'; // Null-terminate buffer
   

   In case of a response greater or equal to sizeof(buffer) the final bufferPosition++ will increment to sizeof(buffer) and the following buffer[bufferPosition] = '\0' will actually end up in the byte following the actual buffer.

   There actually should be a break inside the else if() branch to avoid the additional increment (and the zero-termination inside that branch can be omitted due to the same instruction following the loop.