Skip Menu |
 

History Show all quoted text — Show full headers
# Mon Dec 17 18:47:57 2012 Nickolai Zeldovich <nickolai@csail.mit.edu> - Ticket created
From: Nickolai Zeldovich <nickolai@csail.mit.edu>
Date: Mon, 17 Dec 2012 18:21:12 -0500
Subject: Some more nits
To: krb5-bugs@mit.edu
Download (untitled) / with headers
text/plain 921B
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.
# Mon Dec 17 19:12:25 2012 Nickolai Zeldovich <nickolai@csail.mit.edu> - Correspondence added
From: Nickolai Zeldovich <nickolai@csail.mit.edu>
Date: Mon, 17 Dec 2012 19:12:03 -0500
Subject: Re: [krbdev.mit.edu #7511] AutoReply: Some more nits
To: rt@krbdev.mit.edu
RT-Send-Cc:
Download (untitled) / with headers
text/plain 2.1KiB
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:
Show quoted text
>
> 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.
>
# Thu Dec 20 14:29:34 2012 ghudson@mit.edu (Greg Hudson) - Given to ghudson@mit.edu (Greg Hudson)
# Thu Dec 20 14:29:34 2012 ghudson@mit.edu (Greg Hudson) - Status changed from 'new' to 'review'
# Thu Dec 20 14:29:34 2012 ghudson@mit.edu (Greg Hudson) - Correspondence added
From: ghudson@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 589B

Fix clntraw_create initialization

clntraw_create has been broken since inception; on the first call, it
would compute invalid values of xdrs and client and dereference them.
Fix that. (This is pretty strong evidence that no one has ever used
it.) Reported by Nickolai Zeldovich <nickolai@csail.mit.edu>.

https://github.com/krb5/krb5/commit/8b43dd0cec3645d64e4eb9f6d0fcfc2a31d1955d
Author: Greg Hudson <ghudson@mit.edu>
Commit: 8b43dd0cec3645d64e4eb9f6d0fcfc2a31d1955d
Branch: master
src/lib/rpc/clnt_raw.c | 18 ++++++++++--------
1 files changed, 10 insertions(+), 8 deletions(-)
# Thu Dec 20 14:29:36 2012 ghudson@mit.edu (Greg Hudson) - Correspondence added
From: ghudson@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 575B

Avoid null dereference in BDB dbtree error case

An error case in __bt_first would deference a null pointer. This is
an old upstream BDB bug. Use a separate variable to hold the result
of mpool_get() until it has been checked. Reported by Nickolai
Zeldovich <nickolai@csail.mit.edu>.

https://github.com/krb5/krb5/commit/f5345bba2a993066f9b886dae491d211ed9be057
Author: Greg Hudson <ghudson@mit.edu>
Commit: f5345bba2a993066f9b886dae491d211ed9be057
Branch: master
src/plugins/kdb/db2/libdb2/btree/bt_seq.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
# Thu Dec 20 14:29:37 2012 ghudson@mit.edu (Greg Hudson) - Correspondence added
From: ghudson@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 606B

Remove inoperative null checks in KDC code

In prepare_error_tgs, kdc_free_rstate, and kdc_fast_handle_error,
remove unnecessary null checks. The callers avoid passing null
states, and in each case we've already dereferenced the pointer
earlier. Reported by Nickolai Zeldovich <nickolai@csail.mit.edu>.

https://github.com/krb5/krb5/commit/41b35299cbfa6a47e93f56344cd2e52dd4418ce6
Author: Greg Hudson <ghudson@mit.edu>
Commit: 41b35299cbfa6a47e93f56344cd2e52dd4418ce6
Branch: master
src/kdc/do_tgs_req.c | 6 ++----
src/kdc/fast_util.c | 4 +---
2 files changed, 3 insertions(+), 7 deletions(-)
# Thu Dec 20 14:29:38 2012 ghudson@mit.edu (Greg Hudson) - Correspondence added
From: ghudson@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 499B

Fix signed overflow check in k5_ucs2s_to_utf8s

Signed overflow must be checked before it happens, since modern
versions of gcc will optimize out checks of the result. Reported by
Nickolai Zeldovich <nickolai@csail.mit.edu>.

https://github.com/krb5/krb5/commit/7506becc0ac70915050e097d673e7647b99347fc
Author: Greg Hudson <ghudson@mit.edu>
Commit: 7506becc0ac70915050e097d673e7647b99347fc
Branch: master
src/util/support/utf8_conv.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
# Fri Oct 11 13:37:51 2013 tlyu (Taylor Yu) - Status changed from 'review' to 'resolved'
# Fri Oct 11 13:37:51 2013 tlyu (Taylor Yu) - Version_Fixed 1.12 added
# Fri Oct 11 14:14:41 2013 tlyu (Taylor Yu) - Subject changed from 'Some more nits' to 'Fix minor int overflow and null pointer problems'