protocol memory overflow protection

muccc · Jul 11, 2020 · 044798c · 044798c
1 parent 26ceaf5
commit 044798c
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 14 deletions.
diff --git a/gtest/protocol_test.cpp b/gtest/protocol_test.cpp
@@ -122,17 +122,63 @@ TEST_F( CProtocolTest, ReadValidKeyValuePairFragments ) {
 }
 
 TEST_F( CProtocolTest, TwoStartBytes ) {
-   // TODO
+
+   std::string keyValuePair = "^Garbage^GetSignals;GET$"; 
+   controlbus.SetReadData( keyValuePair );
+   controlbus.SetReadDataReturnValue( keyValuePair.size() ); 
+
+	ASSERT_EQ( true, test.GetKeyValue( key, value ) );
+	ASSERT_STREQ( key.c_str() , "GetSignals" );
+	ASSERT_STREQ( value.c_str() , "GET" );
 }
 
 TEST_F( CProtocolTest, TwoEndBytes ) {
-   // TODO
+   std::string keyValuePair = "^Garbage^GetSignals;GET$Garbage$"; 
+   controlbus.SetReadData( keyValuePair );
+   controlbus.SetReadDataReturnValue( keyValuePair.size() ); 
 
+	ASSERT_EQ( true, test.GetKeyValue( key, value ) );
+	ASSERT_STREQ( key.c_str() , "GetSignals" );
+	ASSERT_STREQ( value.c_str() , "GET" );
 }
 
 TEST_F( CProtocolTest, MessageOverflow ) {
-   // TODO
 
+   std::string fragment1 = "^Fragment1_has_23_bytes"; 
+   std::string fragment2 = "!!!!!!!!!"; // 9 bytes for no overflow
+   std::string fragment3 = ";GET$"; // 5 bytes 
+
+   // positiv test
+   controlbus.SetReadData( fragment1 );
+   controlbus.SetReadDataReturnValue( fragment1.size() ); 
+	ASSERT_EQ( false, test.GetKeyValue( key, value ) );
+
+   controlbus.SetReadData( fragment2 );
+   controlbus.SetReadDataReturnValue( fragment2.size() ); 
+	ASSERT_EQ( false, test.GetKeyValue( key, value ) );
+
+   controlbus.SetReadData( fragment3 );
+   controlbus.SetReadDataReturnValue( fragment3.size() ); 
+	ASSERT_EQ( true, test.GetKeyValue( key, value ) );
+
+   // overflow test 
+
+   controlbus.SetReadData( fragment1 );
+   controlbus.SetReadDataReturnValue( fragment1.size() ); 
+	ASSERT_EQ( false, test.GetKeyValue( key, value ) );
+
+   // now we provoke a overflow 
+   fragment2 += " "; // fragment1 + fragment2 > 32 bytes
+   controlbus.SetReadData( fragment2 );
+   controlbus.SetReadDataReturnValue( fragment2.size() ); 
+	ASSERT_EQ( false, test.GetKeyValue( key, value ) );
+
+   // previous overflow deleted the previous message bytes
+   controlbus.SetReadData( fragment3 );
+   controlbus.SetReadDataReturnValue( fragment3.size() ); 
+	ASSERT_EQ( false, test.GetKeyValue( key, value ) ); // message was to long
+
+
 }
 
 

diff --git a/helper.cpp b/helper.cpp
@@ -4,9 +4,46 @@
 
 #include "interface/helper.h"
 
+#include <iostream>
+
 namespace siguni::helper
 {
 
+
+
+   int CSignalStrings::FindFirstCharacter( const std::string & attSignalMessage, 
+                                           const char attCharacter,
+                                           const size_t attPos )
+   {
+      auto pos = attSignalMessage.find( attCharacter, attPos );
+      if ( std::string::npos == pos )
+      {
+         return -1;
+      }
+      else
+      {
+         return pos;
+      }
+   }
+
+   int CSignalStrings::FindLastCharacter( const std::string & attSignalMessage, 
+                                          const char attCharacter )
+   {
+      int pos = -1;
+      int lastPos { -1 };
+
+      do
+      {
+         pos = FindFirstCharacter( attSignalMessage, attCharacter, pos+1 );
+         if( pos >= 0 )
+         {
+            lastPos = pos;
+         }
+      }while( pos >= 0 );
+
+      return lastPos;
+   }
+
    bool CSignalStrings::ExtractKeyValue( const std::string & attSignalMessage, 
                                          const char attSplitCharacter,
                                          std::string & attKey,

diff --git a/interface/helper.h b/interface/helper.h
@@ -18,10 +18,17 @@ namespace siguni::helper
 
    public: 
 
+      static int FindFirstCharacter( const std::string & attSignalMessage, 
+                                     const char attCharacter,
+                                     const size_t attPos = 0 );
+
+      static int FindLastCharacter( const std::string & attSignalMessage, 
+                                    const char attCharacter );
+
       static bool ExtractKeyValue( const std::string & attSignalMessage, 
-                            const char attSplitCharacter,
-                            std::string & attKey,
-                            std::string & attValue );
+                                   const char attSplitCharacter,
+                                   std::string & attKey,
+                                   std::string & attValue );
 
       static std::vector<std::string_view> 
       GetValueItems( const std::string & attString, const char attSplitCharacter );    

diff --git a/protocol.cpp b/protocol.cpp
@@ -28,9 +28,9 @@ namespace siguni
       // add read bytes to internal message buffer
       messageBuffer += buffer;
 
-      // search start byte within messagebuffer, if not found then clear messagebuffer 
-      auto pos = messageBuffer.find( STARTBYTE );
-      if ( std::string::npos == pos )
+      // search startbyte within messagebuffer, if not found then clear messagebuffer 
+      auto pos = helper::CSignalStrings::FindFirstCharacter( messageBuffer, STARTBYTE ); 
+      if( pos < 0 )
       {
          messageBuffer.clear(); 
          return false;
@@ -39,10 +39,17 @@ namespace siguni
       // remove all bytes left from startbyte
       messageBuffer = messageBuffer.substr( pos ); 
 
-      // search endbyte 
-      pos = messageBuffer.find( ENDBYTE );
-      if ( std::string::npos == pos ) // no endbyte found
+      // search first endbyte after first startbyte
+      pos = helper::CSignalStrings::FindFirstCharacter( messageBuffer, ENDBYTE ); 
+      if( pos < 0 )
       {
+         // we have a max size of input messages 
+         std::cout << messageBuffer.size() << std::endl;
+         if( messageBuffer.size() > MESSAGE_BUFFER_MAX_SIZE )
+         {
+            // clear buffer to prevent a memory overflow for invalid messages 
+            messageBuffer.clear(); 
+         }
          return false;
       }
 
@@ -52,8 +59,12 @@ namespace siguni
       // remove this message from buffer 
       messageBuffer = messageBuffer.substr( pos+1 );
 
-      // check are there more than one startbytes ?     
-
+      // if there are more than one startbytes then delete alle previous startbytes     
+      pos = helper::CSignalStrings::FindLastCharacter( protocolString, STARTBYTE ); 
+      if( pos >= 0)
+      {
+         protocolString = protocolString.substr( pos+1 );
+      }
 
       return helper::CSignalStrings::ExtractKeyValue( 
                      protocolString, SEPARATOR, attKey, attValue ); 

diff --git a/protocol.h b/protocol.h
@@ -25,6 +25,8 @@ namespace siguni
 
       private:
 
+         static constexpr int  MESSAGE_BUFFER_MAX_SIZE { 32 };
+
          static constexpr char STARTBYTE { '^' };
          static constexpr char SEPARATOR { ';' };
          static constexpr char ENDBYTE   { '$' };