Browse Source

checking for buffer overflow in insertChars function

prozessorkern 3 years ago
parent
commit
c1f42d8239
2 changed files with 74 additions and 14 deletions
  1. 19 14
      src/shellmatta_utils.c
  2. 55 0
      test/unittest/shellmatta_utils/test_utils_insertChars.cpp

+ 19 - 14
src/shellmatta_utils.c

@@ -162,46 +162,51 @@ void utils_forwardCursor(shellmatta_instance_t *inst, uint32_t length)
  * @param[in]   inst    pointer to shellmatta instance
  * @param[in]   data    pointer to the data to be inserted
  * @param[in]   length  length of the data to be inserted
- * @todo        this function shall check buffer overflows
  */
 void utils_insertChars( shellmatta_instance_t   *inst,
                         char                    *data,
                         uint32_t                 length)
 {
-    if(0u != length)
+    uint32_t tmpLength = length;
+    /** -# limit the length to the space left in the buffer */
+    if((inst->inputCount + tmpLength) > inst->bufferSize)
     {
+        tmpLength = inst->bufferSize - inst->inputCount;
+    }
+
+    if(0u != tmpLength)
+    {
+
         /** -# check if we have to move chars in the buffer */
         if(     (inst->cursor     != inst->inputCount)
             &&  (SHELLMATTA_MODE_INSERT == inst->mode))
         {
             /** -# move the existing chars */
-            for (   uint32_t i = inst->inputCount;
-                    i > inst->cursor;
-                    i --)
+            for (uint32_t i = inst->inputCount; i > inst->cursor; i --)
             {
-                inst->buffer[i + length - 1] = inst->buffer[i - 1];
+                inst->buffer[i + tmpLength - 1] = inst->buffer[i - 1];
             }
 
             /** -# store and print the new chars */
-            memcpy(&(inst->buffer[inst->cursor]), data, length);
-            utils_writeEcho(inst, data, length);
+            memcpy(&(inst->buffer[inst->cursor]), data, tmpLength);
+            utils_writeEcho(inst, data, tmpLength);
 
             /** -# print the other chars and restore the cursor to this position */
             utils_eraseLine(inst);
             utils_saveCursorPos(inst);
             utils_writeEcho(  inst,
-                        &(inst->buffer[inst->cursor + length]),
+                        &(inst->buffer[inst->cursor + tmpLength]),
                         inst->inputCount - inst->cursor);
             utils_restoreCursorPos(inst);
-            inst->cursor        += length;
-            inst->inputCount    += length;
+            inst->cursor        += tmpLength;
+            inst->inputCount    += tmpLength;
         }
         /** -# overwrite - if the cursor reaches the end of the input it is pushed further */
         else
         {
-            memcpy(&(inst->buffer[inst->cursor]), data, length);
-            utils_writeEcho(inst, data, length);
-            inst->cursor        += length;
+            memcpy(&(inst->buffer[inst->cursor]), data, tmpLength);
+            utils_writeEcho(inst, data, tmpLength);
+            inst->cursor += tmpLength;
             if(inst->cursor > inst->inputCount)
             {
                 inst->inputCount    =  inst->cursor;

+ 55 - 0
test/unittest/shellmatta_utils/test_utils_insertChars.cpp

@@ -123,3 +123,58 @@ TEST_CASE( "shellmatta_insertChars 0 length" ) {
     CHECK( memcmp("\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", write_data, sizeof(write_data) ) == 0u );
     REQUIRE( memcmp("abcdefghij\0\0\0\0\0\0\0\0\0", buffer, sizeof(buffer)) == 0);
 }
+
+TEST_CASE( "shellmatta_insertChars buffer full" ) {
+
+    shellmatta_instance_t inst;
+    char buffer[20] = "abcdefghij\0\0\0\0\0\0\0\0\0";
+
+    memset(&inst, 0, sizeof(inst));
+    inst.buffer = buffer;
+    inst.bufferSize = 20;
+    inst.cursor = 8;
+    inst.inputCount = 10;
+    inst.echoEnabled = true;
+
+    inst.write = writeFct;
+
+    write_callCnt = 0u;
+    memset(write_data, 0, sizeof(write_data));
+    write_idx = 0u;
+
+    utils_insertChars(&inst, (char*)"blksdflsd kfjlk", 10u);
+
+    CHECK( inst.cursor == 18u );
+    CHECK( inst.inputCount == 20u );
+    CHECK( write_callCnt == 5u );
+    CHECK( memcmp("blksdflsd ", write_data, 10u ) == 0u );
+    REQUIRE( memcmp("abcdefghblksdflsd ij", buffer, sizeof(buffer)) == 0);
+}
+
+
+TEST_CASE( "shellmatta_insertChars buffer overflow by 1" ) {
+
+    shellmatta_instance_t inst;
+    char buffer[20] = "abcdefghij\0\0\0\0\0\0\0\0\0";
+
+    memset(&inst, 0, sizeof(inst));
+    inst.buffer = buffer;
+    inst.bufferSize = 20;
+    inst.cursor = 8;
+    inst.inputCount = 10;
+    inst.echoEnabled = true;
+
+    inst.write = writeFct;
+
+    write_callCnt = 0u;
+    memset(write_data, 0, sizeof(write_data));
+    write_idx = 0u;
+
+    utils_insertChars(&inst, (char*)"blksdflsd kfjlk", 11u);
+
+    CHECK( inst.cursor == 18u );
+    CHECK( inst.inputCount == 20u );
+    CHECK( write_callCnt == 5u );
+    CHECK( memcmp("blksdflsd ", write_data, 10u ) == 0u );
+    REQUIRE( memcmp("abcdefghblksdflsd ij", buffer, sizeof(buffer)) == 0);
+}