[921] in Kerberos_V5_Development
decode_krb5_* leak memory on bad packets
daemon@ATHENA.MIT.EDU (Mark W. Eichin)
Mon Sep 11 23:06:14 1995
Date: Mon, 11 Sep 95 23:04:24 -0400
From: "Mark W. Eichin" <eichin@MIT.EDU>
To: krbdev@MIT.EDU
In tracing a leak in kpcd, I noticed a syndrome in decode_*. Note this
fragment from src/lib/krb5/asn.1/krb5_decode.c:
krb5_error_code decode_krb5_ticket(code, rep)
const krb5_data * code;
krb5_ticket ** rep;
{
setup();
alloc_field(*rep,krb5_ticket);
check_apptag(1);
...
alloc_field will calloc() the space for krb5_ticket. check_apptag, if
it fails, will return, without cleaning up the data allocated by
alloc_field.
*none* of the calls to decode_krb5_* routines actually free the
argument in case of failure... and the "spec" for these routines
doesn't suggest that it is the client's responsibility in any case.
/*
krb5_error_code decode_krb5_structure(const krb5_data *code,
krb5_structure **rep);
requires Expects **rep to not have been allocated;
a new *rep is allocated regardless of the old value.
effects Decodes *code into **rep.
Returns ENOMEM if memory is exhausted.
Returns asn1 and krb5 errors.
*/
A possible solution is to add a rep and type (or name of free
function) argument to the check_apptag macro, and change it to call
the appropriate top-level free routine in case of failure. This makes
the macros a bit messier, but perhaps not too much. Comments? In
particular, should the decode_krb5_* functions change, or the callers?
_Mark_