Skip to content

xfrm/sa: bounds-check algo key length and ctx/bmp lengths in parser#456

Open
rawrmonster17 wants to merge 1 commit into
thom311:mainfrom
rawrmonster17:fix/xfrm-sa-parser-oob
Open

xfrm/sa: bounds-check algo key length and ctx/bmp lengths in parser#456
rawrmonster17 wants to merge 1 commit into
thom311:mainfrom
rawrmonster17:fix/xfrm-sa-parser-oob

Conversation

@rawrmonster17

Copy link
Copy Markdown

Summary

xfrmnl_sa_parse() computes allocation and copy sizes from length fields
embedded in the netlink attribute payload (alg_key_len, ctx_len, bmp_len)
without comparing them against the actual attribute length returned by nla_len().

The nla_policy for these attributes sets only .minlen, never .maxlen, so
nlmsg_parse() cannot enforce an upper bound on attribute payload size. This
means a caller supplying an attribute whose payload is shorter than the claimed
key/context/bitmap length causes:

  • OOB heap read (all algo/sec_ctx/ESN paths): the calloc destination is
    sized to the attacker-controlled value, and memcpy() reads that many bytes
    past the end of the (shorter) source attribute, dragging adjacent heap contents
    into sa->aead/auth/crypt/comp/sec_ctx/replay_state_esn.

  • OOB heap write (XFRMA_ALG_AUTH, line 905): the old
    strcpy(sa->auth->alg_name, auth->alg_name) had no NUL-termination
    guarantee on the 64-byte source field, allowing a write past the alg_name
    destination in the freshly allocated sa->auth.

  • Integer overflow (XFRMA_REPLAY_ESN_VAL): bmp_len is uint32_t, and
    sizeof(uint32_t) * bmp_len can wrap to a small value, producing an
    undersized allocation followed by a large memcpy.

Fix

For every variable-length attribute block, validate the payload-embedded length
field against nla_len() before computing the allocation size:

int attr_len = nla_len(tb[XFRMA_ALG_AEAD]);
int key_bytes;

if (attr_len < (int)sizeof(struct xfrm_algo_aead))
    return -NLE_INVAL;
key_bytes = (aead->alg_key_len + 7) / 8;
if (key_bytes > attr_len - (int)sizeof(struct xfrm_algo_aead))
    return -NLE_INVAL;
len = sizeof(struct xfrmnl_algo_aead) + key_bytes;

This pattern is applied to all seven affected blocks:
XFRMA_ALG_AEAD, XFRMA_ALG_AUTH_TRUNC, XFRMA_ALG_AUTH,
XFRMA_ALG_CRYPT, XFRMA_ALG_COMP, XFRMA_SEC_CTX, XFRMA_REPLAY_ESN_VAL.

For XFRMA_ALG_AUTH, strcpy is replaced with a bounded memcpy into the
calloc'd (zero-initialised) alg_name field:

memcpy(sa->auth->alg_name, auth->alg_name, sizeof(sa->auth->alg_name) - 1);

For XFRMA_REPLAY_ESN_VAL, an explicit overflow guard is added before the
bmp_len multiply:

if (esn->bmp_len > (UINT32_MAX / sizeof(uint32_t)))
    return -NLE_INVAL;

Normal well-formed SA messages from the kernel are unaffected: the validated
key_bytes/bmp_bytes values are identical to what the old code computed
whenever the attribute is correctly sized.

Context

This was noted in issue #453, which identified the strcpy on line 905. This
patch addresses the full family of unbounded copies in the same function.
The setter paths were hardened in an earlier PR; this hardens the corresponding
parser/receive path.

xfrmnl_sa_parse() computes allocation and copy sizes from length fields
embedded in the netlink attribute payload (alg_key_len, ctx_len,
bmp_len) without comparing them against the actual attribute length
returned by nla_len().  The nla_policy for these attributes sets only
.minlen, not .maxlen, so nlmsg_parse() cannot enforce an upper bound.

This means a caller supplying an attribute whose payload is shorter
than the claimed key/context/bitmap length causes:

  - An OOB heap read: the calloc destination is sized to the attacker-
    controlled value, and the subsequent memcpy() reads that many bytes
    past the end of the (shorter) source attribute, dragging adjacent
    heap contents into sa->aead/auth/crypt/comp/sec_ctx/replay_state_esn.

  - An OOB heap write on the XFRMA_ALG_AUTH path (line 905): the old
    strcpy(sa->auth->alg_name, auth->alg_name) had no NUL-termination
    guarantee on the 64-byte source field, allowing a write past the
    alg_name destination.

  - An integer overflow on the XFRMA_REPLAY_ESN_VAL path: bmp_len is
    a uint32_t, and sizeof(uint32_t)*bmp_len can wrap to a small value,
    producing an undersized allocation followed by a large copy.

Fix all seven affected attribute blocks by validating the payload-
embedded length field against nla_len() before computing the allocation
size:

  key_bytes = (alg_key_len + 7) / 8;
  if (key_bytes > nla_len(tb[...]) - sizeof(header struct))
      return -NLE_INVAL;

For XFRMA_ALG_AUTH, replace strcpy with a bounded memcpy into the
calloc'd (zero-initialised) alg_name field.

For XFRMA_REPLAY_ESN_VAL, add an explicit overflow guard before the
bmp_len multiply.

Normal well-formed SA messages from the kernel are unaffected: the
validated key_bytes / bmp_bytes values are identical to what the old
code computed whenever the attribute is correctly sized.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant