[921] in Kerberos_V5_Development

home help back first fref pref prev next nref lref last post

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_

home help back first fref pref prev next nref lref last post