From 80f16c62010570342c7ca295744d119caf4fb0e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Rop=C3=A9?= Date: Wed, 16 May 2012 15:29:35 +0200 Subject: [PATCH] Small fixes from static analysis: - Potential NULL dereference in tsmf_ifman.c - Check return value for our functions in tsmf_media.c and rdp.c - Bad binary operator used in gcc.c - Unreachable code in ntlm.c - Bad free operation on SCOPE_LIST object in license.c --- channels/drdynvc/tsmf/tsmf_ifman.c | 12 ++++++++---- channels/drdynvc/tsmf/tsmf_media.c | 1 + libfreerdp-core/gcc.c | 2 +- libfreerdp-core/license.c | 10 +++++++++- libfreerdp-core/rdp.c | 3 ++- libwinpr-sspi/NTLM/ntlm.c | 12 ------------ 6 files changed, 21 insertions(+), 19 deletions(-) diff --git a/channels/drdynvc/tsmf/tsmf_ifman.c b/channels/drdynvc/tsmf/tsmf_ifman.c index 10d351d7b..881b18f94 100644 --- a/channels/drdynvc/tsmf/tsmf_ifman.c +++ b/channels/drdynvc/tsmf/tsmf_ifman.c @@ -128,7 +128,8 @@ int tsmf_ifman_on_new_presentation(TSMF_IFMAN* ifman) presentation = tsmf_presentation_new(stream_get_tail(ifman->input), ifman->channel_callback); if (presentation == NULL) error = 1; - tsmf_presentation_set_audio_device(presentation, ifman->audio_name, ifman->audio_device); + else + tsmf_presentation_set_audio_device(presentation, ifman->audio_name, ifman->audio_device); ifman->output_pending = true; return error; } @@ -389,9 +390,12 @@ int tsmf_ifman_on_end_of_stream(TSMF_IFMAN* ifman) presentation = tsmf_presentation_find_by_id(stream_get_tail(ifman->input)); stream_seek(ifman->input, 16); stream_read_uint32(ifman->input, StreamId); - stream = tsmf_stream_find_by_id(presentation, StreamId); - tsmf_stream_end(stream); - + if (presentation) + { + stream = tsmf_stream_find_by_id(presentation, StreamId); + if (stream) + tsmf_stream_end(stream); + } DEBUG_DVC("StreamId %d", StreamId); stream_check_size(ifman->output, 16); diff --git a/channels/drdynvc/tsmf/tsmf_media.c b/channels/drdynvc/tsmf/tsmf_media.c index 40f9701f2..eac7106af 100644 --- a/channels/drdynvc/tsmf/tsmf_media.c +++ b/channels/drdynvc/tsmf/tsmf_media.c @@ -448,6 +448,7 @@ static void tsmf_sample_playback(TSMF_SAMPLE* sample) sample->pixfmt = pixfmt; } + ret = false ; if (stream->decoder->GetDecodedDimension) ret = stream->decoder->GetDecodedDimension(stream->decoder, &width, &height); if (ret && (width != stream->width || height != stream->height)) diff --git a/libfreerdp-core/gcc.c b/libfreerdp-core/gcc.c index c73a8747e..8db352fe6 100644 --- a/libfreerdp-core/gcc.c +++ b/libfreerdp-core/gcc.c @@ -1131,7 +1131,7 @@ boolean gcc_read_client_cluster_data(STREAM* s, rdpSettings* settings, uint16 bl stream_read_uint32(s, flags); /* flags */ - if ((flags | REDIRECTED_SESSIONID_FIELD_VALID)) + if ((flags & REDIRECTED_SESSIONID_FIELD_VALID)) stream_read_uint32(s, settings->redirected_session_id); /* redirectedSessionID */ return true; diff --git a/libfreerdp-core/license.c b/libfreerdp-core/license.c index 05604230e..5f5a555ba 100644 --- a/libfreerdp-core/license.c +++ b/libfreerdp-core/license.c @@ -579,11 +579,19 @@ void license_free_scope_list(SCOPE_LIST* scopeList) { uint32 i; + /* + * We must NOT call license_free_binary_blob() on each scopelist->array[i] element, + * because scopelist->array was allocated at once, by a single call to xmalloc. The elements + * it contains cannot be deallocated separately then. + * To make things clean, we must deallocate each scopelist->array[].data, + * and finish by deallocating scopelist->array with a single call to xfree(). + */ for (i = 0; i < scopeList->count; i++) { - license_free_binary_blob(&scopeList->array[i]); + xfree(scopeList->array[i].data); } + xfree(scopeList->array) ; xfree(scopeList); } diff --git a/libfreerdp-core/rdp.c b/libfreerdp-core/rdp.c index dd839210b..b11ab2727 100644 --- a/libfreerdp-core/rdp.c +++ b/libfreerdp-core/rdp.c @@ -224,7 +224,8 @@ boolean rdp_read_header(rdpRdp* rdp, STREAM* s, uint16* length, uint16* channel_ enum DomainMCSPDU MCSPDU; MCSPDU = (rdp->settings->server_mode) ? DomainMCSPDU_SendDataRequest : DomainMCSPDU_SendDataIndication; - mcs_read_domain_mcspdu_header(s, &MCSPDU, length); + if (!mcs_read_domain_mcspdu_header(s, &MCSPDU, length)) + return false ; if (*length - 8 > stream_get_left(s)) return false; diff --git a/libwinpr-sspi/NTLM/ntlm.c b/libwinpr-sspi/NTLM/ntlm.c index c29c6531b..dbfbdc665 100644 --- a/libwinpr-sspi/NTLM/ntlm.c +++ b/libwinpr-sspi/NTLM/ntlm.c @@ -296,9 +296,6 @@ SECURITY_STATUS SEC_ENTRY ntlm_AcceptSecurityContext(PCredHandle phCredential, P { context->state = NTLM_STATE_NEGOTIATE; - if (!context) - return SEC_E_INVALID_HANDLE; - if (!pInput) return SEC_E_INVALID_TOKEN; @@ -338,9 +335,6 @@ SECURITY_STATUS SEC_ENTRY ntlm_AcceptSecurityContext(PCredHandle phCredential, P } else if (context->state == NTLM_STATE_AUTHENTICATE) { - if (!context) - return SEC_E_INVALID_HANDLE; - if (!pInput) return SEC_E_INVALID_TOKEN; @@ -436,12 +430,6 @@ SECURITY_STATUS SEC_ENTRY ntlm_InitializeSecurityContextA(PCredHandle phCredenti } else { - if (!context) - return SEC_E_INVALID_HANDLE; - - if (!pInput) - return SEC_E_INVALID_TOKEN; - if (pInput->cBuffers < 1) return SEC_E_INVALID_TOKEN;