[17273] in Kerberos_V5_Development
Re: [PATCH 3/4] Use gssalloc_malloc()/gssalloc_free() with
daemon@ATHENA.MIT.EDU (Greg Hudson)
Thu Oct 6 15:08:52 2011
From: Greg Hudson <ghudson@mit.edu>
To: Sam Hartman <hartmans@painless-security.com>
In-Reply-To: <1317921933-22035-4-git-send-email-hartmans@painless-security.com>
Date: Thu, 06 Oct 2011 15:08:48 -0400
Message-ID: <1317928128.1548.26.camel@t410>
Mime-Version: 1.0
Cc: Kevin Wasserman <kevin.wasserman@painless-security.com>,
"krbdev@mit.edu" <krbdev@mit.edu>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: krbdev-bounces@mit.edu
On Thu, 2011-10-06 at 13:25 -0400, Sam Hartman wrote:
> @@ -495,6 +496,7 @@ sign_server(int s, gss_cred_id_t server_creds, int export)
> }
> } else {
> msg_buf = xmit_buf;
> + xmit_buf.value = 0;
This seems broken. msg_buf is supposed to contain the plain text of the
message when this block is done. Instead, this causes msg_buf to be
invalid (leading to a crash at least if logfile is set), and xmit_buf's
memory to be leaked.
> + if (msg_buf.value) {
> + if (token_flags & TOKEN_WRAPPED) {
> + gss_release_buffer(&min_stat, &msg_buf);
> + }
> + else {
> free(msg_buf.value);
> msg_buf.value = 0;
> }
> + }
Tracking the allocation status of a buffer using a superficially
unrelated variable is a maintenance trap. Keep separately owned memory
in separate variables. That is, make a recv_buf object to hold the
locally allocated received-message buffer and an unwrap_buf object to
possibly hold the unwrapped value. Then use a pointer to decide which
buffer to use as input for sending.
_______________________________________________
krbdev mailing list krbdev@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev
