Incorret data read

[tpiat]

I received but can not consistently reproduce the bug. When I read from server incorrect data, the program crashes. (The server of open62451 do not give similar error) Received data has incorrect dst->length (as I think), next I sometime receive SIGSEGV. For example, dst->length = 8001096 in function UA_encodeBinary.

Is data integrity checks?

[jpfr]

Hey @tpiat,

In order to help you, we will either need

• a stack trace from a debugger
• a dump of the network traffic, where the bug occurs. This can be obtained e.g. with wireshark

[tpiat]

I'll try to get dump tomorrow. But this is very rary error, and the dump will be very big or I must select a tail of sequence. Also wireshark can't get traffic to 127.0.0.1. Probably, it is best to insert a record of traffic to program. How can I do it?

IMHO, Stack is destroyed, because any function call after this error crashes my application.

[Stasik0]

@tpiat: wireshark can get localhost traffic in linux, get a VM e.g. with ubuntu. Please also let the client run in a debugget to get the begtrace. Did I understood witght, that only the client is the problem, right? Server has a far more protected stack, while client is not protected, yet (https://github.com/acplt/open62541/blob/master/src/client/ua_client.c#L269).

[tpiat]

I write a client. Client works with open62541 server without any problems. This problem rarely occur with Simatic server (by Siemens) I never know about server part of open62541.

[Stasik0]

please get us a dump (just the tail of will be enough), we will need to to harden ua_client.c a bit to check for field lengths

[ichrispa]

@tpiat: Please post instructions to reproduce your error (code), including error logs and a backtrace.

You can insert a Loopback adapter on microsoft windows over the add devices system control panel which is capturable with wireshark.

Received data has incorrect dst->length (as I think), next I sometime receive SIGSEGV. For example, dst->length = 8001096 in function UA_encodeBinary.

This may just as well be an unitialized or wrongly copied struct/pointer that you are passing to the stack. Please post the code that can reproduce this problem.

[tpiat]

The code the same as #340

#include "open62541.h"
#include <string>

using namespace std;

struct OPCUA_Nodes
{
    string ip;
    UA_Client *client;
    int client_connected;
    UA_ReadRequest *rReq;
};

OPCUA_Nodes node;

void core_Logger(UA_LogLevel level, UA_LogCategory category, const char *msg, ...)
{   
}

int localget()
{
    string uri("opc.tcp://");
    uri+=node.ip;

    if(!node.client_connected)
    {
        UA_StatusCode retval = UA_Client_connect(node.client, ClientNetworkLayerTCP_connect,(char *)uri.c_str());
        if(retval != UA_STATUSCODE_GOOD)
        {
            UA_Client_delete(node.client);
            node.client = UA_Client_new(UA_ClientConfig_standard, core_Logger);
            node.client_connected=0;
            return retval;
        }
        else node.client_connected=1;
    }

    UA_ReadResponse rResp = UA_Client_read(node.client, node.rReq);
    if(rResp.responseHeader.serviceResult != UA_STATUSCODE_GOOD)
    {
        node.client_connected=0;
    }
    UA_ReadResponse_deleteMembers(&rResp);

    return 0;
}

void mainLoop()
{
    while (1)
    {
        localget();
//      sleepMicro(1000000); // Sleep 1s
    }
}

int main(int argc, char* argv[])
{
    node.ip="127.0.0.1:4845";
    node.client_connected=0;
    node.client = UA_Client_new(UA_ClientConfig_standard, core_Logger);
    node.rReq = new UA_ReadRequest;
    UA_ReadRequest_init(node.rReq);
    node.rReq->nodesToRead = (UA_ReadValueId *)UA_Array_new(&UA_TYPES[UA_TYPES_READVALUEID], 1);
    node.rReq->nodesToReadSize = 1;
    node.rReq->nodesToRead[0].nodeId = UA_NODEID_STRING_ALLOC(3, "DEMO.statepath.alarm.SourceName"); // assume this node exists

    mainLoop();
    return 0;
}

[tpiat]

Stack:

open62541.c 3233    stack   Frame #14
open62541.c 3106    stack   Frame #15
open62541.c 3083    stack   Frame #16
open62541.c 3162    stack   Frame #17
open62541.c 3083    stack   Frame #18
open62541.c 3229    stack   Frame #19
open62541.c 3106    stack   Frame #20
open62541.c 3083    stack   Frame #21
open62541.c 3229    stack   Frame #22
open62541.c 3106    stack   Frame #23
open62541.c 3083    stack   Frame #24
open62541.c 11347   stack   Frame #25
open62541.c 11467   stack   Frame #26
main.cpp    27  stack   Frame #27
main.cpp    52  stack   Frame #28
main.cpp    68  stack   Frame #29

And also 14 records with system dll or ??

[Stasik0]

and you are using master HEAD amalgamated file? (i try to reconstruct the function names from the line numbers)

[tpiat]

This is true dump of stack

open62541.c 3453    stack   Frame #0 
(Fn UA_UInt64_encodeBinary, *(UA_UInt64*) &dst->data[*offset] = *src;)
open62541.c 4208    stack   Frame #1
(Fn UA_encodeBinary, retval = UA_UInt64_encodeBinary((const UA_UInt64*) ptr, dst, offset);)
open62541.c 4248    stack   Frame #2
(Fn UA_encodeBinary, retval = UA_encodeBinary((const void*) ptr, memberType, dst, offset);)
open62541.c 5819    stack   Frame #3
(UA_SecureChannel_sendBinaryMessage, retval |= UA_encodeBinary(content, contentType, &message, &messagePos);)
open62541.c 11215   stack   Frame #4
(Fn synchronousRequest, UA_StatusCode retval = UA_SecureChannel_sendBinaryMessage(&client->channel, requestId,)
open62541.c 11493   stack   Frame #5
(Fn UA_Client_read, synchronousRequest(client, request, &UA_TYPES[UA_TYPES_READREQUEST], &response,)
main.cpp    38  stack   Frame #6
main.cpp    52  stack   Frame #7
main.cpp    68  stack   Frame #8

I use not original file, I added variavle z and also

if(dst->length>65536)
{
    retval++;
}

lines. Add 5 after line 4297.

***** open62541.c
 4296:              // UA_TYPES_XMLELEMENT
 4297:              retval = UA_encodeBinary((const void*) ptr, memberType, dst, offset);
***** MyOPEN62541.C
 4296:              // UA_TYPES_XMLELEMENT
 4297:                          if(dst->length>65536)
 4298:                          {
 4299:                                  retval++;
 4300:                          }
 4301:
 4302:              retval = UA_encodeBinary((const void*) ptr, memberType, dst, offset);
*****

I have add it for breakepoint in line 4299

[tpiat]

Last received, Len=65

4D 53 47 46 41 00 00 00 71 47 02 00 01 00 00 00 C7 93 01 00 95 93 01 00 01 00 7A 02 3E 0C ED BB D2 DF D0 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 02 00 00 35 80 00 00 00 00

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.