Skip to content

Commit

Permalink
gsskrb5: CVE-2022-3437 Avoid undefined behaviour in _gssapi_verify_pad()
Browse files Browse the repository at this point in the history
By decrementing 'pad' only when we know it's safe, we ensure we can't
stray backwards past the start of a buffer, which would be undefined
behaviour.

In the previous version of the loop, 'i' is the number of bytes left to
check, and 'pad' is the current byte we're checking. 'pad' was
decremented at the end of each loop iteration. If 'i' was 1 (so we
checked the final byte), 'pad' could potentially be pointing to the
first byte of the input buffer, and the decrement would put it one
byte behind the buffer.

That would be undefined behaviour.

The patch changes it so that 'pad' is the byte we previously checked,
which allows us to ensure that we only decrement it when we know we
have a byte to check.

Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
  • Loading branch information
jsutton24 authored and nicowilliams committed Nov 4, 2022
1 parent a587a4b commit c758910
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions lib/gssapi/krb5/decapsulate.c
Expand Up @@ -193,13 +193,13 @@ _gssapi_verify_pad(gss_buffer_t wrapped_token,
if (wrapped_token->length < 1)
return GSS_S_BAD_MECH;

pad = (u_char *)wrapped_token->value + wrapped_token->length - 1;
padlength = *pad;
pad = (u_char *)wrapped_token->value + wrapped_token->length;
padlength = pad[-1];

if (padlength > datalen)
return GSS_S_BAD_MECH;

for (i = padlength; i > 0 && *pad == padlength; i--, pad--)
for (i = padlength; i > 0 && *--pad == padlength; i--)
;
if (i != 0)
return GSS_S_BAD_MIC;
Expand Down

0 comments on commit c758910

Please sign in to comment.