Skip to content

STUN: address-of-pointer copy after stun_atoaddr() overwrites the resolved server address #335

Description

@garacil

Summary

In libsofia-sip-ua/stun/stun.c, both stun_test_lifetime() and stun_get_nattype(), on the branch where a server address string is supplied, copy from &sd->sd_pri_info.ai_addr right after calling stun_atoaddr(). That source is the address of the ai_addr pointer field inside the su_addrinfo_t, not the socket address it points to. The copy overwrites the resolved server address held in sd->sd_pri_addr with su_addrinfo_t tail bytes (the pointer value and the fields after it). Because sd_pri_info sits immediately before sd_pri_addr in struct stun_discovery_s, the two regions also overlap.

Why the copy is wrong and unnecessary

stun_discovery_create() aims ai_addr at the destination buffer:

stun.c:1148   sd->sd_pri_info.ai_addr = &sd->sd_pri_addr->su_sa;

stun_atoaddr() then writes the resolved address through that pointer:

stun.c:2623   dstaddr = (su_sockaddr_t *) info->ai_addr;
   ...         memcpy(&dstaddr->su_sa, res->ai_addr, sizeof(struct sockaddr));

So when stun_atoaddr() returns, sd->sd_pri_addr already contains the resolved server address. The follow-up copy is redundant, and by reading &sd->sd_pri_info.ai_addr it clobbers that address with unrelated bytes.

Affected sites:

stun.c:2740  (stun_test_lifetime)  memcpy (sd->sd_pri_addr, &sd->sd_pri_info.ai_addr, sizeof(su_sockaddr_t));
stun.c:1251  (stun_get_nattype)    memmove(sd->sd_pri_addr, &sd->sd_pri_info.ai_addr, sizeof(su_sockaddr_t));

GCC 14 at -O2 also reports the overlap at the memcpy site:

stun.c:2740: warning: 'memcpy' accessing 32 bytes at offsets 96 and 72 overlaps 8 bytes at offset 96 [-Wrestrict]

The memmove site does not warn (memmove permits overlap) but carries the same semantic bug.

Proposed fix

Remove the redundant copy in both server branches; stun_atoaddr() already wrote the resolved address into sd_pri_addr. The no-server branch's copy from sh_pri_addr is a genuine copy between two different objects and stays unchanged. A patch follows as a pull request.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions