added a testcase that replays client messages from a file

fixed the first round of bugs found by file-based fuzzing with afl

include/ua_server.h

@@ -125,8 +125,9 @@ typedef struct UA_WorkItem {
  *
  * @return Upon sucess, UA_STATUSCODE_GOOD is returned. An error code otherwise.
  */
-UA_StatusCode UA_EXPORT UA_Server_addTimedWorkItem(UA_Server *server, const UA_WorkItem *work,
-                                                   UA_DateTime executionTime, UA_Guid *resultWorkGuid);
+UA_StatusCode UA_EXPORT
+UA_Server_addTimedWorkItem(UA_Server *server, const UA_WorkItem *work,
+                           UA_DateTime executionTime, UA_Guid *resultWorkGuid);
 
 /**
  * @param server The server object.
@@ -144,8 +145,9 @@ UA_StatusCode UA_EXPORT UA_Server_addTimedWorkItem(UA_Server *server, const UA_W
  *
  * @return Upon sucess, UA_STATUSCODE_GOOD is returned. An error code otherwise.
  */
-UA_StatusCode UA_EXPORT UA_Server_addRepeatedWorkItem(UA_Server *server, const UA_WorkItem *work,
-                                                      UA_UInt32 interval, UA_Guid *resultWorkGuid);
+UA_StatusCode UA_EXPORT
+UA_Server_addRepeatedWorkItem(UA_Server *server, const UA_WorkItem *work,
+                              UA_UInt32 interval, UA_Guid *resultWorkGuid);
 
 /** Remove timed or repeated work */
 /* UA_Boolean UA_EXPORT UA_Server_removeWorkItem(UA_Server *server, UA_Guid workId); */

src/server/ua_server_binary.c

@@ -68,29 +68,33 @@ static void processOPN(UA_Connection *connection, UA_Server *server, const UA_By
     }
 
     UA_UInt32 secureChannelId;
-    UA_UInt32_decodeBinary(msg, pos, &secureChannelId);
+    UA_StatusCode retval = UA_UInt32_decodeBinary(msg, pos, &secureChannelId);
 
     UA_AsymmetricAlgorithmSecurityHeader asymHeader;
-    UA_AsymmetricAlgorithmSecurityHeader_decodeBinary(msg, pos, &asymHeader);
+    retval |= UA_AsymmetricAlgorithmSecurityHeader_decodeBinary(msg, pos, &asymHeader);
 
     UA_SequenceHeader seqHeader;
-    UA_SequenceHeader_decodeBinary(msg, pos, &seqHeader);
+    retval |= UA_SequenceHeader_decodeBinary(msg, pos, &seqHeader);
 
     UA_NodeId requestType;
-    UA_NodeId_decodeBinary(msg, pos, &requestType);
+    retval |= UA_NodeId_decodeBinary(msg, pos, &requestType);
+
+    UA_OpenSecureChannelRequest r;
+    retval |= UA_OpenSecureChannelRequest_decodeBinary(msg, pos, &r);
 
-    if(requestType.identifier.numeric != 446) {
+    if(retval != UA_STATUSCODE_GOOD || requestType.identifier.numeric != 446) {
+        UA_AsymmetricAlgorithmSecurityHeader_deleteMembers(&asymHeader);
+        UA_SequenceHeader_deleteMembers(&seqHeader);
+        UA_NodeId_deleteMembers(&requestType);
+        UA_OpenSecureChannelRequest_deleteMembers(&r);
         connection->close(connection);
         return;
     }
 
-    UA_OpenSecureChannelRequest r;
     UA_OpenSecureChannelResponse p;
-    UA_OpenSecureChannelRequest_decodeBinary(msg, pos, &r);
     UA_OpenSecureChannelResponse_init(&p);
     Service_OpenSecureChannel(server, connection, &r, &p);
 
-    /* Response */
     UA_SecureConversationMessageHeader respHeader;
     respHeader.messageHeader.messageTypeAndFinal = UA_MESSAGETYPEANDFINAL_OPNF;
     respHeader.messageHeader.messageSize = 0;
@@ -108,7 +112,7 @@ static void processOPN(UA_Connection *connection, UA_Server *server, const UA_By
 
     UA_ByteString resp_msg = (UA_ByteString){
         .length = respHeader.messageHeader.messageSize,
-            .data = UA_alloca(respHeader.messageHeader.messageSize)
+        .data = UA_alloca(respHeader.messageHeader.messageSize)
     };
 
     size_t tmpPos = 0;

src/server/ua_server_worker.c

@@ -526,7 +526,8 @@ UA_StatusCode UA_Server_run(UA_Server *server, UA_UInt16 nThreads, UA_Boolean *r
                 pthread_cond_broadcast(&server->dispatchQueue_condition); 
 #else
             processWork(server, work, workSize);
-            UA_free(work);
+            if(workSize > 0)
+                UA_free(work);
 #endif
         }
 

src/ua_types.c

@@ -88,8 +88,10 @@ void UA_String_init(UA_String *p) {
 
 UA_TYPE_DELETE_DEFAULT(UA_String)
 void UA_String_deleteMembers(UA_String *p) {
-	if(p->data)
+	if(p->data) {
 		UA_free(p->data);
+        p->data = UA_NULL;
+    }
 }
 
 UA_StatusCode UA_String_copy(UA_String const *src, UA_String *dst) {
@@ -332,20 +334,12 @@ static UA_Boolean UA_NodeId_isBasicType(UA_NodeId const *id) {
 UA_TYPE_DELETE_DEFAULT(UA_NodeId)
 void UA_NodeId_deleteMembers(UA_NodeId *p) {
     switch(p->identifierType) {
-    case UA_NODEIDTYPE_NUMERIC:
-        // nothing to do
-        break;
-    case UA_NODEIDTYPE_STRING: // Table 6, second entry
-        UA_String_deleteMembers(&p->identifier.string);
-        break;
-
-    case UA_NODEIDTYPE_GUID: // Table 6, third entry
-        UA_Guid_deleteMembers(&p->identifier.guid);
-        break;
-
-    case UA_NODEIDTYPE_BYTESTRING: // Table 6, "OPAQUE"
+    case UA_NODEIDTYPE_STRING:
+    case UA_NODEIDTYPE_BYTESTRING:
         UA_ByteString_deleteMembers(&p->identifier.byteString);
         break;
+    default:
+        break;
     }
 }
 
@@ -1006,6 +1000,7 @@ void UA_deleteMembers(void *p, const UA_DataType *dataType) {
             UA_Int32 noElements = *((UA_Int32*)ptr);
             ptr += sizeof(UA_Int32) + (member->padding & 0x07);
             UA_Array_delete(*(void**)ptr, memberType, noElements);
+            *(void**)ptr = UA_NULL;
             ptr += sizeof(void*);
             continue;
         }

tests/CMakeLists.txt

@@ -44,3 +44,22 @@ add_test(nodestore ${CMAKE_CURRENT_BINARY_DIR}/check_nodestore)
 # add_executable(check_startup check_startup.c)
 # target_link_libraries(check_startup ${LIBS})
 # add_test(startup ${CMAKE_CURRENT_BINARY_DIR}/check_startup)
+
+# test with canned interactions from files
+
+add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/client_HELOPN.bin
+                   PRE_BUILD
+                   COMMAND python ${PROJECT_SOURCE_DIR}/tools/hex2bin.py ${CMAKE_CURRENT_SOURCE_DIR}/dumps/client_HELOPN.hex
+                   DEPENDS ${PROJECT_SOURCE_DIR}/tools/hex2bin.py
+                           ${CMAKE_CURRENT_SOURCE_DIR}/dumps/client_HELOPN.hex)
+
+add_executable(check_server_interaction_fileinput check_server_interaction_fileinput.c
+                                                  testing_networklayers.c
+                                                  ${PROJECT_SOURCE_DIR}/examples/logger_stdout.c
+                                                  $<TARGET_OBJECTS:open62541-object>)
+
+target_include_directories(check_server_interaction_fileinput PRIVATE ${PROJECT_SOURCE_DIR}/examples)
+target_include_directories(check_server_interaction_fileinput PRIVATE ${PROJECT_BINARY_DIR})
+target_link_libraries(check_server_interaction_fileinput ${LIBS})
+add_test(server_interaction_fileinput ${CMAKE_CURRENT_BINARY_DIR}/check_server_interaction_fileinput ${CMAKE_CURRENT_BINARY_DIR}/client_HELOPN.bin)
+add_custom_target(server_interaction_fileinput ALL DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/client_HELOPN.bin)

tests/check_server_interaction_fileinput.c

@@ -0,0 +1,64 @@
+#include <stdlib.h>
+#include "check.h"
+#ifdef NOT_AMALGATED
+    #include "ua_server.h"
+#else
+    #include "open62541.h"
+#endif
+#include "testing_networklayers.h"
+#include "logger_stdout.h"
+
+#include <stdio.h>
+
+UA_Boolean running = 1;
+UA_UInt32 read_count = 0;
+UA_UInt32 max_reads;
+char **filenames;
+
+static void writeCallback(void *handle, UA_ByteStringArray buf) {
+}
+
+static void readCallback(void) {
+    read_count++;
+    if(read_count > max_reads)
+        running = UA_FALSE;
+}
+
+START_TEST(readVariable) {
+	UA_Server *server = UA_Server_new();
+    UA_Server_setLogger(server, Logger_Stdout_new());
+    UA_Server_addNetworkLayer(server, ServerNetworkLayerFileInput_new(max_reads, filenames, readCallback,
+                                                                      writeCallback, NULL));
+    UA_StatusCode retval = UA_Server_run(server, 1, &running);
+	UA_Server_delete(server);
+
+	ck_assert_int_eq(retval, UA_STATUSCODE_GOOD);
+}
+END_TEST
+
+static Suite *testSuite_builtin(void) {
+	Suite *s = suite_create("Test server with client messages stored in text files");
+
+	TCase *tc_nano = tcase_create("nano profile");
+	tcase_add_test(tc_nano, readVariable);
+	suite_add_tcase(s, tc_nano);
+
+	return s;
+}
+
+int main(int argc, char **argv) {
+    if(argc < 2)
+        return EXIT_FAILURE;
+    filenames = &argv[1];
+    max_reads = argc - 1;
+	int      number_failed = 0;
+	Suite   *s;
+	SRunner *sr;
+	s  = testSuite_builtin();
+	sr = srunner_create(s);
+	srunner_set_fork_status(sr, CK_NOFORK);
+	srunner_run_all(sr, CK_NORMAL);
+	number_failed += srunner_ntests_failed(sr);
+	srunner_free(sr);
+	return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}

tests/dumps/client_HELOPN.hex

@@ -0,0 +1,13 @@
+48 45 4c 46 38 00 00 00 00 00 00 00 00 00 01 00  /* HELF8........... */
+00 00 01 00 00 00 00 01 88 13 00 00 18 00 00 00  /* ................ */
+6f 70 63 2e 74 63 70 3a 2f 2f 6c 6f 63 61 6c 68  /* opc.tcp://localh */
+6f 73 74 3a 34 38 34 32                          /* ost:4842         */
+4f 50 4e 46 85 00 00 00 00 00 00 00 2f 00 00 00  /* OPNF......../... */
+68 74 74 70 3a 2f 2f 6f 70 63 66 6f 75 6e 64 61  /* http://opcfounda */
+74 69 6f 6e 2e 6f 72 67 2f 55 41 2f 53 65 63 75  /* tion.org/UA/Secu */
+72 69 74 79 50 6f 6c 69 63 79 23 4e 6f 6e 65 ff  /* rityPolicy#None. */
+ff ff ff ff ff ff ff 33 00 00 00 01 00 00 00 01  /* .......3........ */
+00 be 01 00 00 97 db 02 02 a8 d2 ce 01 00 00 00  /* ................ */
+00 00 00 00 00 ff ff ff ff 00 00 00 00 00 00 00  /* ................ */
+00 00 00 00 00 00 00 00 01 00 00 00 01 00 00 00  /* ................ */
+00 e0 93 04 00                                   /* .....            */

tests/testing_networklayers.c

@@ -0,0 +1,99 @@
+#include <unistd.h>
+#include <fcntl.h>
+#include <malloc.h>
+#include <assert.h>
+#include "testing_networklayers.h"
+
+typedef struct {
+	UA_Connection connection;
+    UA_UInt32 files;
+    char **filenames;
+    UA_UInt32 files_read;
+    void (*writeCallback)(void *, UA_ByteStringArray buf);
+    void (*readCallback)(void);
+    void *callbackHandle;
+} NetworkLayer_FileInput;
+
+/** Accesses only the sockfd in the handle. Can be run from parallel threads. */
+static void writeCallback(NetworkLayer_FileInput *handle, UA_ByteStringArray gather_buf) {
+    handle->writeCallback(handle->callbackHandle, gather_buf);
+}
+
+static void closeCallback(NetworkLayer_FileInput *handle) {
+}
+
+static UA_StatusCode NetworkLayer_FileInput_start(NetworkLayer_FileInput *layer, UA_Logger *logger) {
+    return UA_STATUSCODE_GOOD;
+}
+
+static UA_Int32
+NetworkLayer_FileInput_getWork(NetworkLayer_FileInput *layer, UA_WorkItem **workItems, UA_UInt16 timeout)
+{
+    layer->readCallback();
+
+    // open a new connection
+    // return a single buffer with the entire file
+
+    if(layer->files >= layer->files_read)
+        return 0;
+    
+    int filefd = open(layer->filenames[layer->files_read], O_RDONLY);
+    layer->files_read++;
+    if(filefd == -1)
+        return 0;
+
+    UA_Byte *buf = malloc(layer->connection.localConf.maxMessageSize);
+    UA_Int32 bytes_read = read(filefd, buf, layer->connection.localConf.maxMessageSize);
+    close(filefd);
+    if(bytes_read <= 0) {
+        free(buf);
+        return 0;
+    }
+        
+    *workItems = malloc(sizeof(UA_WorkItem));
+    UA_WorkItem *work = *workItems;
+    work->type = UA_WORKITEMTYPE_BINARYNETWORKMESSAGE;
+    work->work.binaryNetworkMessage.connection = &layer->connection;
+    work->work.binaryNetworkMessage.message = (UA_ByteString){.length = bytes_read, .data = (UA_Byte*)buf};
+
+    return 1;
+}
+
+static UA_Int32 NetworkLayer_FileInput_stop(NetworkLayer_FileInput * layer, UA_WorkItem **workItems) {
+    // remove the connection in the server
+    // return removeAllConnections(layer, workItems);
+    return 0;
+}
+
+static void NetworkLayer_FileInput_delete(NetworkLayer_FileInput *layer) {
+	free(layer);
+}
+
+
+UA_ServerNetworkLayer
+ServerNetworkLayerFileInput_new(UA_UInt32 files, char **filenames, void(*readCallback)(void),
+                                void(*writeCallback) (void*, UA_ByteStringArray buf),
+                                void *callbackHandle)
+{
+    NetworkLayer_FileInput *layer = malloc(sizeof(NetworkLayer_FileInput));
+    layer->connection.state = UA_CONNECTION_OPENING;
+    layer->connection.localConf = UA_ConnectionConfig_standard;
+    layer->connection.channel = (void*)0;
+    layer->connection.close = (void (*)(void*))closeCallback;
+    layer->connection.write = (void (*)(void*, UA_ByteStringArray))writeCallback;
+
+    layer->files = files;
+    layer->filenames = filenames;
+    layer->files_read = 0;
+    layer->readCallback = readCallback;
+    layer->writeCallback = writeCallback;
+    layer->callbackHandle = callbackHandle;
+    
+    UA_ServerNetworkLayer nl;
+    nl.nlHandle = layer;
+    nl.start = (UA_StatusCode (*)(void*, UA_Logger *logger))NetworkLayer_FileInput_start;
+    nl.getWork = (UA_Int32 (*)(void*, UA_WorkItem**, UA_UInt16)) NetworkLayer_FileInput_getWork;
+    nl.stop = (UA_Int32 (*)(void*, UA_WorkItem**)) NetworkLayer_FileInput_stop;
+    nl.free = (void (*)(void*))NetworkLayer_FileInput_delete;
+    return nl;
+}

tests/testing_networklayers.h

@@ -0,0 +1,16 @@
+#ifndef TESTING_NETWORKLAYERS_H_
+#define TESTING_NETWORKLAYERS_H_
+
+#ifdef NOT_AMALGATED
+    #include "ua_server.h"
+#else
+    #include "open62541.h"
+#endif
+
+/** @brief Create the TCP networklayer and listen to the specified port */
+UA_ServerNetworkLayer
+ServerNetworkLayerFileInput_new(UA_UInt32 files, char **filenames, void(*readCallback)(void),
+                                void(*writeCallback) (void*, UA_ByteStringArray buf),
+                                void *callbackHandle);
+
+#endif /* TESTING_NETWORKLAYERS_H_ */

tools/hex2bin.py

@@ -0,0 +1,23 @@
+import sys
+import os
+import binascii
+import re
+
+def clean_line(string):
+    comment_re = re.compile("/\*.*?\*/") # matches C-style comments /* */ at the end of a line
+    return re.sub(comment_re, "" ,string).replace(' ','').replace('\n','')
+
+if len(sys.argv) < 2:
+    print("Usage: python hex2bin.py file1.hex file2.hex ...")
+    exit(0)
+
+filenames = sys.argv[1:]
+for f in filenames:
+    bn = os.path.basename(f)
+    with open(f) as ff:
+        with open(bn[:-4] + ".bin", 'w') as out:
+            lines = ff.readlines()
+            for l in lines:
+                c = clean_line(l)
+                out.write(binascii.unhexlify(c))
+