Skip to content

Memory leak in TCP/IP Interface Object attribute 5 and 6 setters on the validation-failure path #605

Description

@Mayur021

Summary

DecodeCipTcpIpInterfaceConfiguration (attribute 5) and DecodeCipTcpIpInterfaceHostName
(attribute 6) allocate a CIP string from the request and then return on the validation-failure
path without freeing it. The allocation is made into a function-local struct, so the pointer is
lost. Each rejected request leaks one allocation, and the requests are unauthenticated.

Verified against master 8c8fa9f.

Detail

source/src/cip/ciptcpipinterface.c:443 (attribute 5):

SetCipStringByData(&if_cfg.domain_name, domain_name_length,
                   message_router_request->data);

SetCipStringByData allocates via CipCalloc (cip/cipstring.c:179). Then at line 450:

if (!IsValidNetworkConfig(&if_cfg)
        || (domain_name_length > 0
                && !IsValidDomain(if_cfg.domain_name.string))) {
    message_router_response->general_status = kCipErrorInvalidAttributeValue;
    return number_of_decoded_bytes;      /* if_cfg.domain_name.string is never freed */
}

if_cfg is a local, so on this return the allocation is unreachable.

The same shape is at ciptcpipinterface.c:486 and the return at :494 for the host name,
using the local tmp_host_name.

The only CipFree calls on these strings are at lines 687 and 692, in the shutdown path.

Reproduction

Build with OPENER_TCPIP_IFACE_CFG_SETTABLE=1 and AddressSanitizer, then over one session:

  1. RegisterSession
  2. SetAttributeSingle class 0xF5 instance 1 attribute 3 = 0 (static IP; attribute 5 will not
    decode otherwise)
  3. SetAttributeSingle class 0xF5 instance 1 attribute 5, declaring a domain name length of 48
    that fails IsValidDomain

Repeat step 3, then send SIGINT. LeakSanitizer after roughly 100 requests:

SUMMARY: AddressSanitizer: 4944 byte(s) leaked in 103 allocation(s)

4944 / 103 = 48 bytes, matching the declared length exactly: one lost allocation per request.

Note that RSS is not a useful instrument here, since 48 bytes per request is well below page
granularity.

Impact

Unauthenticated remote memory leak, up to 48 bytes per request on attribute 5 and 64 on
attribute 6, unbounded and repeatable. On the memory-constrained devices OpENer typically runs
on, sustained requests will exhaust the heap. It is resource exhaustion rather than memory
corruption.

This requires OPENER_TCPIP_IFACE_CFG_SETTABLE, which is 0 in the sample configurations but is
enabled for DLR devices and is described in opener_user_conf.h as required by ODVA
publication 70.

Suggested fix

Release the string on each failure return, using the existing helper:

ClearCipString(&if_cfg.domain_name);      /* before the attribute 5 error return */
ClearCipString(&tmp_host_name);           /* before the attribute 6 error return */

I have not sent a pull request, since #586 is open against these same functions and would
conflict; whoever lands that may prefer to fold this in.

Prior art checked

Searched on 2026-08-07 across open and closed issues and pull requests for ciptcpipinterface,
domain_name, SetCipStringByData, memory leak, TCP/IP object, CipCalloc:

I may still have missed something. If this duplicates a report you already have, say so and I
will close it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions