Merge pull request #1133 from open62541/hotfix/fuzzing_2690

Fix fuzzing issue 2690

CMakeLists.txt

@@ -116,6 +116,8 @@ option(UA_BUILD_UNIT_TESTS "Build the unit tests" OFF)
 option(UA_BUILD_FUZZING "Build the fuzzing executables" OFF)
 option(UA_BUILD_OSS_FUZZ "Special build switch used in oss-fuzz" OFF)
 mark_as_advanced(UA_BUILD_OSS_FUZZ)
+option(UA_DEBUG_DUMP_PKGS "Dump every package received by the server as hexdump format" OFF)
+mark_as_advanced(UA_DEBUG_DUMP_PKGS)
 option(UA_BUILD_EXAMPLES_NODESET_COMPILER "Generate an OPC UA information model from a nodeset XML (experimental)" OFF)
 
 # Advanced Build Targets
@@ -339,6 +341,10 @@ set(default_plugin_sources ${PROJECT_SOURCE_DIR}/plugins/ua_network_tcp.c
                            ${PROJECT_SOURCE_DIR}/plugins/ua_log_stdout.c
                            ${PROJECT_SOURCE_DIR}/plugins/ua_accesscontrol_default.c
                            ${PROJECT_SOURCE_DIR}/plugins/ua_config_standard.c)
+if(UA_DEBUG_DUMP_PKGS)
+    list(APPEND lib_sources ${PROJECT_SOURCE_DIR}/plugins/ua_debug_dump_pkgs.c)
+endif()
+
 if(UA_ENABLE_EMBEDDED_LIBC)
   list(APPEND lib_sources ${PROJECT_SOURCE_DIR}/deps/libc_string.c)
 endif()

include/ua_config.h.in

@@ -40,6 +40,7 @@ extern "C" {
 #cmakedefine UA_ENABLE_DISCOVERY
 #cmakedefine UA_ENABLE_DISCOVERY_MULTICAST
 #cmakedefine UA_ENABLE_DISCOVERY_SEMAPHORE
+#cmakedefine UA_DEBUG_DUMP_PKGS
 
 /**
  * Standard Includes

plugins/ua_debug_dump_pkgs.c

@@ -0,0 +1,31 @@
+/* This work is licensed under a Creative Commons CCZero 1.0 Universal License.
+ * See http://creativecommons.org/publicdomain/zero/1.0/ for more information. */
+
+#include "ua_util.h"
+
+#include <ctype.h>
+#include <stdio.h>
+void UA_dump_hex_pkg(UA_Byte* buffer, size_t bufferLen) {
+    printf("--------------- HEX Package Start ---------------\n");
+    char ascii[17];
+    memset(ascii,0,17);
+    for (size_t i = 0; i < bufferLen; i++)
+    {
+        if (i == 0)
+            printf("%08zx ", i);
+        else if (i%16 == 0)
+            printf("|%s|\n%08zx ", ascii, i);
+        if (isprint((int)(buffer[i])))
+            ascii[i%16] = (char)buffer[i];
+        else
+            ascii[i%16] = '.';
+        printf("%02X ", (unsigned char)buffer[i]);
+    }
+    size_t fillPos = bufferLen %16;
+    ascii[fillPos] = 0;
+    for (size_t i=fillPos; i<16; i++) {
+        printf("   ");
+    }
+    printf("|%s|\n%08zx\n", ascii, bufferLen);
+    printf("--------------- HEX Package END ---------------\n");
+}

src/server/ua_server_binary.c

@@ -615,6 +615,11 @@ processBinaryMessage(UA_Server *server, UA_Connection *connection,
                      UA_ByteString *message) {
     UA_LOG_TRACE(server->config.logger, UA_LOGCATEGORY_NETWORK,
                  "Connection %i | Received a packet.", connection->sockfd);
+
+    #ifdef UA_DEBUG_DUMP_PKGS
+    UA_dump_hex_pkg(message->data, message->length);
+    #endif
+
     UA_Boolean realloced = UA_FALSE;
     UA_StatusCode retval = UA_Connection_completeChunks(connection, message, &realloced);
 

src/ua_connection.c

@@ -58,6 +58,12 @@ completeChunksUntil(UA_Connection *connection, UA_ByteString * UA_RESTRICT messa
             break;
         }
 
+        UA_Byte isFinal = message->data[complete_until+3];
+        if (isFinal != 'C' && isFinal != 'F' && isFinal != 'A') {
+            *garbage_end = true; /* the message type is not recognized */
+            break;
+        }
+
         /* Decoding failed or the message size is not allowed. The remaining
          * message is garbage. */
         UA_UInt32 chunk_length = 0;
@@ -125,7 +131,7 @@ UA_StatusCode
 UA_Connection_completeChunks(UA_Connection *connection,
                              UA_ByteString * UA_RESTRICT message,
                              UA_Boolean * UA_RESTRICT realloced) {
-    /* If we have a stored an incomplete chunk, prefix to the received message.
+    /* If we have stored an incomplete chunk, prefix to the received message.
      * After this block, connection->incompleteMessage is always empty. The
      * message and the buffer is released if allocating the memory fails. */
     if(connection->incompleteMessage.length > 0) {

src/ua_util.h

@@ -135,6 +135,10 @@ size_t UA_readNumber(u8 *buf, size_t buflen, u32 *number);
 # define UA_TYPENAME(name)
 #endif
 
+#ifdef UA_DEBUG_DUMP_PKGS
+void UA_EXPORT UA_dump_hex_pkg(UA_Byte* buffer, size_t bufferLen);
+#endif
+
 #ifdef __cplusplus
 } // extern "C"
 #endif

tests/fuzz/binary.dict

@@ -24,4 +24,13 @@ header_hel_chunk="HELC"
 header_ack_chunk="ACKC"
 header_clo_chunk="CLOC"
 
+# Message header for message abort (see Spec Part 6, Table 26)
+
+header_msg_abort="MSGA"
+header_err_abort="ERRA"
+header_opn_abort="OPNA"
+header_hel_abort="HELA"
+header_ack_abort="ACKA"
+header_clo_abort="CLOA"
+
 # TODO add dict for Security Header and Sequence Header

tests/fuzz/fuzz_binary_message.cc

@@ -18,16 +18,17 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
     UA_ServerConfig config = UA_ServerConfig_standard;
     config.logger = UA_Log_Stdout;
     UA_Server *server = UA_Server_new(config);
-    UA_ByteString msg = {
-			size, //length
-			const_cast<UA_Byte*>(data) //data
-	};
 
-    config.logger = UA_Log_Stdout;
-    UA_Boolean reallocated = UA_FALSE;
-    UA_StatusCode retval = UA_Connection_completeChunks(&c, &msg, &reallocated);
-    if(retval == UA_STATUSCODE_GOOD && msg.length > 0)
-        UA_Server_processBinaryMessage(server, &c, &msg);
+    // we need to copy the message because it will be freed in the processing function
+    UA_ByteString msg = UA_ByteString();
+    UA_StatusCode retval = UA_ByteString_allocBuffer(&msg, size);
+    if(retval != UA_STATUSCODE_GOOD)
+        return (int)retval;
+    memcpy(msg.data, data, size);
+
+    UA_Server_processBinaryMessage(server, &c, &msg);
+	// if we got an invalid chunk, the message is not deleted, so delete it here
+    UA_ByteString_deleteMembers(&msg);
     UA_Server_delete(server);
     UA_Connection_deleteMembers(&c);
     return 0;

tests/testing_networklayers.c

@@ -15,7 +15,7 @@ dummyGetSendBuffer(UA_Connection *connection, size_t length, UA_ByteString *buf)
 
 static void
 dummyReleaseSendBuffer(UA_Connection *connection, UA_ByteString *buf) {
-    UA_free(buf->data);
+    UA_ByteString_delete(buf);
 }
 
 static UA_StatusCode
@@ -26,7 +26,7 @@ dummySend(UA_Connection *connection, UA_ByteString *buf) {
 
 static void
 dummyReleaseRecvBuffer(UA_Connection *connection, UA_ByteString *buf) {
-    return;
+    UA_ByteString_deleteMembers(buf);
 }
 
 static void