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

Open Beginner friendly
#605 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
68/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
c

Research direction

Start in source/src/cip/ciptcpipinterface.c at the attribute 5 and 6 setters, then review SetCipStringByData and ClearCipString in cip/cipstring.c. Build with OPENER_TCPIP_IFACE_CFG_SETTABLE=1 and AddressSanitizer, reproduce the invalid domain or host-name requests, and verify that both validation-failure paths no longer report leaks.

Written by the indexing model from the issue text.

Description

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:

  • #601 ("Fix: Memory leaks"), merged: changes are in ports/generic_networkhandler.c and free
    socket error-message strings; does not touch this file.
  • #586 ("Fix OOB read in TCP/IP interface hostname validation"), open: rewrites
    IsValidNameLabel and IsValidDomain to take explicit lengths. It adds no frees, so this leak
    remains after it is applied.
  • #569 and #567 are separate out-of-bounds issues in other paths.

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

Dominant language
C
Stars
858
Forks
314
Avg merge
18d 2h
Merged PRs (30d)
1

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from EIPStackGroup/OpENer

All issues in EIPStackGroup/OpENer

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.