mirrors/FreeRDP
0
0
Fork 0
mirror of https://github.com/FreeRDP/FreeRDP.git synced 2024-11-27 11:54:44 +08:00
Code Issues Packages Projects Releases Wiki Activity

Additional checks (#8279)

Browse Source 
* Fix length checks in parallel driver

The length requested was not checked against the length read from
the port.

* Fixed missing length check in video channel

Data received in video redirection channel was not checked for
proper length.

* Fixed video presentation reference counter

Video channel presentation reference counter was not updated with
Video_Frame_new. A failing H264 decoding step could trigger a reference
decrement and the presentation was freed by accident.
Also clean up the increment and decrement of presentation

* Fixed tsmf ffmpeg context extra data size checks

tsmf_ffmpeg_init_stream did not abort if the video format ExtraDataSize
was not sufficiently large to satisfy expectations.

* Fixed missing input data length check

tsmf_ifman_exchange_capability_request did not check if the input
data stream actually contained the amount of bytes requested to
copy.

* Fixed TSMF tsmf_ffmpeg_set_format length checks

ExtraDataSize of format was not checked for expected minimum length

* Fixed TSMF tsmf_read_format_type length checks

ExtraDataSize of format was not checked for expected minimum
length

* Fixed TSMF tsmf_gstreamer_set_format length checks

ExtraDataSize of format was not checked for expected minimum
length
This commit is contained in:
akallabeth 2022-10-06 15:30:54 +02:00 committed by GitHub
parent 229e047231
commit 26ac2f0b27
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 84 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
channels/parallel/client/parallel_main.c
View File

@ -157,7 +157,7 @@ static UINT parallel_process_irp_read(PARALLEL_DEVICE* parallel, IRP* irp)
return ERROR_INVALID_DATA; return ERROR_INVALID_DATA;
Stream_Read_UINT32(irp->input, Length); Stream_Read_UINT32(irp->input, Length);
Stream_Read_UINT64(irp->input, Offset); Stream_Read_UINT64(irp->input, Offset);
buffer = (BYTE*)malloc(Length); buffer = (BYTE*)calloc(Length, sizeof(BYTE));
if (!buffer) if (!buffer)
{ {
@ -176,6 +176,7 @@ static UINT parallel_process_irp_read(PARALLEL_DEVICE* parallel, IRP* irp)
} }
else else
{ {
Length = status;
} }
Stream_Write_UINT32(irp->output, Length); Stream_Write_UINT32(irp->output, Length);

20
channels/tsmf/client/ffmpeg/tsmf_ffmpeg.c
View File

@ -193,9 +193,12 @@ static BOOL tsmf_ffmpeg_init_stream(ITSMFDecoder* decoder, const TS_AM_MEDIA_TYP
if (media_type->SubType == TSMF_SUB_TYPE_AVC1 && if (media_type->SubType == TSMF_SUB_TYPE_AVC1 &&
media_type->FormatType == TSMF_FORMAT_TYPE_MPEG2VIDEOINFO) media_type->FormatType == TSMF_FORMAT_TYPE_MPEG2VIDEOINFO)
{ {
size_t required = 6;
/* The extradata format that FFmpeg uses is following CodecPrivate in Matroska. /* The extradata format that FFmpeg uses is following CodecPrivate in Matroska.
See http://haali.su/mkv/codecs.pdf */ See http://haali.su/mkv/codecs.pdf */
p = mdecoder->codec_context->extradata; p = mdecoder->codec_context->extradata;
if (mdecoder->codec_context->extradata_size < required)
return FALSE;
*p++ = 1; /* Reserved? */ *p++ = 1; /* Reserved? */
*p++ = media_type->ExtraData[8]; /* Profile */ *p++ = media_type->ExtraData[8]; /* Profile */
*p++ = 0; /* Profile */ *p++ = 0; /* Profile */
@ -204,17 +207,28 @@ static BOOL tsmf_ffmpeg_init_stream(ITSMFDecoder* decoder, const TS_AM_MEDIA_TYP
*p++ = 0xe0 | 0x01; /* Reserved | #sps */ *p++ = 0xe0 | 0x01; /* Reserved | #sps */
s = media_type->ExtraData + 20; s = media_type->ExtraData + 20;
size = ((UINT32)(*s)) * 256 + ((UINT32)(*(s + 1))); size = ((UINT32)(*s)) * 256 + ((UINT32)(*(s + 1)));
required += size + 2;
if (mdecoder->codec_context->extradata_size < required)
return FALSE;
memcpy(p, s, size + 2); memcpy(p, s, size + 2);
s += size + 2; s += size + 2;
p += size + 2; p += size + 2;
required++;
if (mdecoder->codec_context->extradata_size < required)
return FALSE;
*p++ = 1; /* #pps */ *p++ = 1; /* #pps */
size = ((UINT32)(*s)) * 256 + ((UINT32)(*(s + 1))); size = ((UINT32)(*s)) * 256 + ((UINT32)(*(s + 1)));
required += size + 2;
if (mdecoder->codec_context->extradata_size < required)
return FALSE;
memcpy(p, s, size + 2); memcpy(p, s, size + 2);
} }
else else
{ {
memcpy(mdecoder->codec_context->extradata, media_type->ExtraData, memcpy(mdecoder->codec_context->extradata, media_type->ExtraData,
media_type->ExtraDataSize); media_type->ExtraDataSize);
if (mdecoder->codec_context->extradata_size < media_type->ExtraDataSize + 8)
return FALSE;
memset(mdecoder->codec_context->extradata + media_type->ExtraDataSize, 0, 8); memset(mdecoder->codec_context->extradata + media_type->ExtraDataSize, 0, 8);
} }
} }
@ -243,6 +257,9 @@ static BOOL tsmf_ffmpeg_set_format(ITSMFDecoder* decoder, TS_AM_MEDIA_TYPE* medi
{ {
TSMFFFmpegDecoder* mdecoder = (TSMFFFmpegDecoder*)decoder; TSMFFFmpegDecoder* mdecoder = (TSMFFFmpegDecoder*)decoder;
WINPR_ASSERT(mdecoder);
WINPR_ASSERT(media_type);
switch (media_type->MajorType) switch (media_type->MajorType)
{ {
case TSMF_MAJOR_TYPE_VIDEO: case TSMF_MAJOR_TYPE_VIDEO:
@ -295,6 +312,9 @@ static BOOL tsmf_ffmpeg_set_format(ITSMFDecoder* decoder, TS_AM_MEDIA_TYPE* medi
http://msdn.microsoft.com/en-us/library/dd757806.aspx */ http://msdn.microsoft.com/en-us/library/dd757806.aspx */
if (media_type->ExtraData) if (media_type->ExtraData)
{ {
if (media_type->ExtraDataSize < 12)
return FALSE;
media_type->ExtraData += 12; media_type->ExtraData += 12;
media_type->ExtraDataSize -= 12; media_type->ExtraDataSize -= 12;
} }

2
channels/tsmf/client/gstreamer/tsmf_gstreamer.c
View File

@ -414,6 +414,8 @@ static BOOL tsmf_gstreamer_set_format(ITSMFDecoder* decoder, TS_AM_MEDIA_TYPE* m
http://msdn.microsoft.com/en-us/library/dd757806.aspx */ http://msdn.microsoft.com/en-us/library/dd757806.aspx */
if (media_type->ExtraData) if (media_type->ExtraData)
{ {
if (media_type->ExtraDataSize < 12)
return FALSE;
media_type->ExtraData += 12; media_type->ExtraData += 12;
media_type->ExtraDataSize -= 12; media_type->ExtraDataSize -= 12;
} }

7
channels/tsmf/client/tsmf_codec.c
View File

@ -387,7 +387,12 @@ static BOOL tsmf_read_format_type(TS_AM_MEDIA_TYPE* mediatype, wStream* s, UINT3
if (cbFormat > 176) if (cbFormat > 176)
{ {
mediatype->ExtraDataSize = cbFormat - 176; const size_t nsize = cbFormat - 176;
if (mediatype->ExtraDataSize < nsize)
return FALSE;
if (!Stream_CheckAndLogRequiredLength(TAG, s, nsize))
return FALSE;
mediatype->ExtraDataSize = nsize;
mediatype->ExtraData = Stream_Pointer(s); mediatype->ExtraData = Stream_Pointer(s);
} }
break; break;

16
channels/tsmf/client/tsmf_ifman.c
View File

@ -67,16 +67,20 @@ UINT tsmf_ifman_rim_exchange_capability_request(TSMF_IFMAN* ifman)
*/ */
UINT tsmf_ifman_exchange_capability_request(TSMF_IFMAN* ifman) UINT tsmf_ifman_exchange_capability_request(TSMF_IFMAN* ifman)
{ {
UINT32 i; UINT32 i = 0;
UINT32 v; UINT32 v = 0;
UINT32 pos; UINT32 pos = 0;
UINT32 CapabilityType; UINT32 CapabilityType = 0;
UINT32 cbCapabilityLength; UINT32 cbCapabilityLength = 0;
UINT32 numHostCapabilities; UINT32 numHostCapabilities = 0;
WINPR_ASSERT(ifman);
if (!Stream_EnsureRemainingCapacity(ifman->output, ifman->input_size + 4)) if (!Stream_EnsureRemainingCapacity(ifman->output, ifman->input_size + 4))
return ERROR_OUTOFMEMORY; return ERROR_OUTOFMEMORY;
if (Stream_GetRemainingLength(ifman->input) < ifman->input_size)
return ERROR_INVALID_DATA;
pos = Stream_GetPosition(ifman->output); pos = Stream_GetPosition(ifman->output);
Stream_Copy(ifman->input, ifman->output, ifman->input_size); Stream_Copy(ifman->input, ifman->output, ifman->input_size);
Stream_SetPosition(ifman->output, pos); Stream_SetPosition(ifman->output, pos);

73
channels/video/client/video_main.c
View File

@ -106,7 +106,7 @@ struct s_VideoClientContextPriv
PresentationContext* currentPresentation; PresentationContext* currentPresentation;
}; };
static void PresentationContext_unref(PresentationContext* presentation); static void PresentationContext_unref(PresentationContext** presentation);
static void VideoClientContextPriv_free(VideoClientContextPriv* priv); static void VideoClientContextPriv_free(VideoClientContextPriv* priv);
static const char* video_command_name(BYTE cmd) static const char* video_command_name(BYTE cmd)
@ -172,6 +172,14 @@ fail:
return NULL; return NULL;
} }
static BOOL PresentationContext_ref(PresentationContext* presentation)
{
WINPR_ASSERT(presentation);
InterlockedIncrement(&presentation->refCounter);
return TRUE;
}
static PresentationContext* PresentationContext_new(VideoClientContext* video, BYTE PresentationId, static PresentationContext* PresentationContext_new(VideoClientContext* video, BYTE PresentationId,
UINT32 x, UINT32 y, UINT32 width, UINT32 height) UINT32 x, UINT32 y, UINT32 width, UINT32 height)
{ {
@ -217,18 +225,24 @@ static PresentationContext* PresentationContext_new(VideoClientContext* video, B
goto fail; goto fail;
} }
ret->refCounter = 1; if (!PresentationContext_ref(ret))
goto fail;
return ret; return ret;
fail: fail:
PresentationContext_unref(ret); PresentationContext_unref(&ret);
return NULL; return NULL;
} }
static void PresentationContext_unref(PresentationContext* presentation) static void PresentationContext_unref(PresentationContext** ppresentation)
{ {
PresentationContext* presentation;
MAPPED_GEOMETRY* geometry; MAPPED_GEOMETRY* geometry;
WINPR_ASSERT(ppresentation);
presentation = *ppresentation;
if (!presentation) if (!presentation)
return; return;
@ -248,6 +262,7 @@ static void PresentationContext_unref(PresentationContext* presentation)
Stream_Free(presentation->currentSample, TRUE); Stream_Free(presentation->currentSample, TRUE);
presentation->video->deleteSurface(presentation->video, presentation->surface); presentation->video->deleteSurface(presentation->video, presentation->surface);
free(presentation); free(presentation);
*ppresentation = NULL;
} }
static void VideoFrame_free(VideoFrame** pframe) static void VideoFrame_free(VideoFrame** pframe)
@ -265,7 +280,7 @@ static void VideoFrame_free(VideoFrame** pframe)
WINPR_ASSERT(frame->presentation->video); WINPR_ASSERT(frame->presentation->video);
WINPR_ASSERT(frame->presentation->video->priv); WINPR_ASSERT(frame->presentation->video->priv);
BufferPool_Return(frame->presentation->video->priv->surfacePool, frame->surfaceData); BufferPool_Return(frame->presentation->video->priv->surfacePool, frame->surfaceData);
PresentationContext_unref(frame->presentation); PresentationContext_unref(&frame->presentation);
free(frame); free(frame);
*pframe = NULL; *pframe = NULL;
} }
@ -288,7 +303,7 @@ static VideoFrame* VideoFrame_new(VideoClientContextPriv* priv, PresentationCont
goto fail; goto fail;
mappedGeometryRef(geom); mappedGeometryRef(geom);
frame->presentation = presentation;
frame->publishTime = presentation->lastPublishTime; frame->publishTime = presentation->lastPublishTime;
frame->geometry = geom; frame->geometry = geom;
frame->w = surface->alignedWidth; frame->w = surface->alignedWidth;
@ -299,6 +314,10 @@ static VideoFrame* VideoFrame_new(VideoClientContextPriv* priv, PresentationCont
if (!frame->surfaceData) if (!frame->surfaceData)
goto fail; goto fail;
frame->presentation = presentation;
if (!PresentationContext_ref(frame->presentation))
goto fail;
return frame; return frame;
fail: fail:
@ -329,7 +348,7 @@ void VideoClientContextPriv_free(VideoClientContextPriv* priv)
DeleteCriticalSection(&priv->framesLock); DeleteCriticalSection(&priv->framesLock);
if (priv->currentPresentation) if (priv->currentPresentation)
PresentationContext_unref(priv->currentPresentation); PresentationContext_unref(&priv->currentPresentation);
BufferPool_Free(priv->surfacePool); BufferPool_Free(priv->surfacePool);
free(priv); free(priv);
@ -414,14 +433,13 @@ static BOOL video_onMappedGeometryClear(MAPPED_GEOMETRY* geometry)
static UINT video_PresentationRequest(VideoClientContext* video, static UINT video_PresentationRequest(VideoClientContext* video,
const TSMM_PRESENTATION_REQUEST* req) const TSMM_PRESENTATION_REQUEST* req)
{ {
VideoClientContextPriv* priv = video->priv;
PresentationContext* presentation;
UINT ret = CHANNEL_RC_OK; UINT ret = CHANNEL_RC_OK;
WINPR_ASSERT(video); WINPR_ASSERT(video);
WINPR_ASSERT(req); WINPR_ASSERT(req);
presentation = priv->currentPresentation; VideoClientContextPriv* priv = video->priv;
WINPR_ASSERT(priv);
if (req->Command == TSMM_START_PRESENTATION) if (req->Command == TSMM_START_PRESENTATION)
{ {
@ -434,9 +452,9 @@ static UINT video_PresentationRequest(VideoClientContext* video,
return CHANNEL_RC_OK; return CHANNEL_RC_OK;
} }
if (presentation) if (priv->currentPresentation)
{ {
if (presentation->PresentationId == req->PresentationId) if (priv->currentPresentation->PresentationId == req->PresentationId)
{ {
WLog_ERR(TAG, "ignoring start request for existing presentation %" PRIu8, WLog_ERR(TAG, "ignoring start request for existing presentation %" PRIu8,
req->PresentationId); req->PresentationId);
@ -444,8 +462,7 @@ static UINT video_PresentationRequest(VideoClientContext* video,
} }
WLog_ERR(TAG, "releasing current presentation %" PRIu8, req->PresentationId); WLog_ERR(TAG, "releasing current presentation %" PRIu8, req->PresentationId);
PresentationContext_unref(presentation); PresentationContext_unref(&priv->currentPresentation);
presentation = priv->currentPresentation = NULL;
} }
if (!priv->geometry) if (!priv->geometry)
@ -462,24 +479,23 @@ static UINT video_PresentationRequest(VideoClientContext* video,
} }
WLog_DBG(TAG, "creating presentation 0x%x", req->PresentationId); WLog_DBG(TAG, "creating presentation 0x%x", req->PresentationId);
presentation = PresentationContext_new( priv->currentPresentation = PresentationContext_new(
video, req->PresentationId, geom->topLevelLeft + geom->left, video, req->PresentationId, geom->topLevelLeft + geom->left,
geom->topLevelTop + geom->top, req->SourceWidth, req->SourceHeight); geom->topLevelTop + geom->top, req->SourceWidth, req->SourceHeight);
if (!presentation) if (!priv->currentPresentation)
{ {
WLog_ERR(TAG, "unable to create presentation video"); WLog_ERR(TAG, "unable to create presentation video");
return CHANNEL_RC_NO_MEMORY; return CHANNEL_RC_NO_MEMORY;
} }
mappedGeometryRef(geom); mappedGeometryRef(geom);
presentation->geometry = geom; priv->currentPresentation->geometry = geom;
priv->currentPresentation = presentation; priv->currentPresentation->video = video;
presentation->video = video; priv->currentPresentation->ScaledWidth = req->ScaledWidth;
presentation->ScaledWidth = req->ScaledWidth; priv->currentPresentation->ScaledHeight = req->ScaledHeight;
presentation->ScaledHeight = req->ScaledHeight;
geom->custom = presentation; geom->custom = priv->currentPresentation;
geom->MappedGeometryUpdate = video_onMappedGeometryUpdate; geom->MappedGeometryUpdate = video_onMappedGeometryUpdate;
geom->MappedGeometryClear = video_onMappedGeometryClear; geom->MappedGeometryClear = video_onMappedGeometryClear;
@ -490,16 +506,15 @@ static UINT video_PresentationRequest(VideoClientContext* video,
else if (req->Command == TSMM_STOP_PRESENTATION) else if (req->Command == TSMM_STOP_PRESENTATION)
{ {
WLog_DBG(TAG, "stopping presentation 0x%x", req->PresentationId); WLog_DBG(TAG, "stopping presentation 0x%x", req->PresentationId);
if (!presentation) if (!priv->currentPresentation)
{ {
WLog_ERR(TAG, "unknown presentation to stop %" PRIu8, req->PresentationId); WLog_ERR(TAG, "unknown presentation to stop %" PRIu8, req->PresentationId);
return CHANNEL_RC_OK; return CHANNEL_RC_OK;
} }
priv->currentPresentation = NULL;
priv->droppedFrames = 0; priv->droppedFrames = 0;
priv->publishedFrames = 0; priv->publishedFrames = 0;
PresentationContext_unref(presentation); PresentationContext_unref(&priv->currentPresentation);
} }
return ret; return ret;
@ -713,7 +728,7 @@ treat_feedback:
{ {
UINT32 computedRate; UINT32 computedRate;
InterlockedIncrement(&priv->currentPresentation->refCounter); PresentationContext_ref(priv->currentPresentation);
if (priv->droppedFrames) if (priv->droppedFrames)
{ {
@ -774,7 +789,7 @@ treat_feedback:
priv->lastSentRate, priv->publishedFrames, priv->droppedFrames); priv->lastSentRate, priv->publishedFrames, priv->droppedFrames);
} }
PresentationContext_unref(priv->currentPresentation); PresentationContext_unref(&priv->currentPresentation);
} }
WLog_DBG(TAG, "currentRate=%" PRIu32 " published=%" PRIu32 " dropped=%" PRIu32, WLog_DBG(TAG, "currentRate=%" PRIu32 " published=%" PRIu32 " dropped=%" PRIu32,
@ -894,8 +909,6 @@ static UINT video_VideoData(VideoClientContext* context, const TSMM_VIDEO_DATA*
return CHANNEL_RC_OK; return CHANNEL_RC_OK;
} }
InterlockedIncrement(&presentation->refCounter);
EnterCriticalSection(&priv->framesLock); EnterCriticalSection(&priv->framesLock);
enqueueResult = Queue_Enqueue(priv->frames, frame); enqueueResult = Queue_Enqueue(priv->frames, frame);
LeaveCriticalSection(&priv->framesLock); LeaveCriticalSection(&priv->framesLock);
@ -958,6 +971,8 @@ static UINT video_data_on_data_received(IWTSVirtualChannelCallback* pChannelCall
Stream_Read_UINT16(s, data.PacketsInSample); Stream_Read_UINT16(s, data.PacketsInSample);
Stream_Read_UINT32(s, data.SampleNumber); Stream_Read_UINT32(s, data.SampleNumber);
Stream_Read_UINT32(s, data.cbSample); Stream_Read_UINT32(s, data.cbSample);
if (!Stream_CheckAndLogRequiredLength(TAG, s, data.cbSample))
return ERROR_INVALID_DATA;
data.pSample = Stream_Pointer(s); data.pSample = Stream_Pointer(s);
/* /*