MIME-Version: 1.0
Sender: nickolai.zeldovich@gmail.com
In-Reply-To: <rt-7511-37831.13.5226176123577@krbdev.mit.edu>
References: <rt-7511@krbdev.mit.edu> <rt-7511-37831.13.5226176123577@krbdev.mit.edu>
From: Nickolai Zeldovich <nickolai@csail.mit.edu>
Date: Mon, 17 Dec 2012 19:12:03 -0500
Message-ID: <CADGQE7E4avkjmzmwXveBWHeS58ennUZF9n9CmSqEsO=FwR+HwQ@mail.gmail.com>
Subject: Re: [krbdev.mit.edu #7511] AutoReply: Some more nits
To: rt@krbdev.mit.edu
Content-Type: text/plain; charset=UTF-8
RT-Send-Cc: 
Content-Length: 2191

One more nit:

src/lib/rpc/clnt_raw.c:95,96: the first time clntraw_create() is
called, clp will be NULL, and despite checking for this and allocating
memory on lines 98-103, the xdrs and client variables are never
re-computed, which can lead to dereferencing invalid pointers later in
the same function.

[ No code in krb5 actually calls clntraw_create(), and no application
is likely to call it either, because it would immediately crash; I
have no idea how much you care about actually fixing this code vs.
just removing clnt_raw.c. ]

Nickolai.

On Mon, Dec 17, 2012 at 6:47 PM, krb5 <rt@krbdev.mit.edu> wrote:
>
> Greetings,
>
> This message has been automatically generated in response to the
> creation of a trouble ticket regarding:
>         "Some more nits",
> a summary of which appears below.
>
> There is no need to reply to this message right now.  Your ticket has been
> assigned an ID of [krbdev.mit.edu #7511].
>
> Please include the string:
>
>          [krbdev.mit.edu #7511]
>
> in the subject line of all future correspondence about this issue. To do so,
> you may reply to this message.
>
>                         Thank you,
>
>
> -------------------------------------------------------------------------
> On the current git master branch (2af891a5):
>
> src/util/support/utf8_conv.c:281: (len + n < len) is never required to
> be true: len is a signed integer, n is known to be >= 1 (from line
> 279), and overflow for signed integers is undefined behavior.  Many
> compilers (e.g., gcc) will discard the code from lines 281-282 as a
> result.
>
> src/kdc/do_tgs_req.c:884: if state can be NULL, then it should have
> been checked before dereferencing state at line 850.
>
> src/kdc/fast_util.c:300: if s can be NULL, then it should have been
> checked before dereferencing s at line 298.
>
> src/kdc/fast_util.c:421: if state can be NULL, then it should have
> been checked before dereferencing state at line 417.
>
> src/plugins/kdb/db2/libdb2/btree/bt_seq.c:449: h is guaranteed to be
> NULL at this point, but it's being dereferenced anyway.  Perhaps this
> was meant to refer to the previous value of h, before it was
> overwritten on line 447?
>
> Nickolai.
>