Reviewed-by: Jaben Carsey <[email protected]> Was this found by an actual buffer overflow occurring or by an automated test?
> -----Original Message----- > From: Qiu, Shumin > Sent: Friday, May 08, 2015 6:03 AM > To: [email protected]; Carsey, Jaben > Cc: Yao, Jiewen > Subject: RE: [edk2] [PATCH] ShellPkg: Fix buffer overflow issue in 'map' > command. > Importance: High > > Hi Jiewen, > You are right. Patch updated. > > -Shumin > > -----Original Message----- > From: Yao, Jiewen [mailto:[email protected]] > Sent: Friday, May 08, 2015 4:44 PM > To: [email protected]; Carsey, Jaben > Subject: Re: [edk2] [PATCH] ShellPkg: Fix buffer overflow issue in 'map' > command. > > Hello > The 2nd parameter should be: The maximum number of Destination Unicode > char, including terminating null char. > > Should we use "(StrSize(Specific) + sizeof(CHAR16))/sizeof(CHAR16)" > for 2nd parameter? > > Thank you > Yao Jiewen > > > -----Original Message----- > From: Qiu Shumin [mailto:[email protected]] > Sent: Friday, May 08, 2015 4:27 PM > To: [email protected]; Carsey, Jaben > Subject: [edk2] [PATCH] ShellPkg: Fix buffer overflow issue in 'map' > command. > > This patch replace 'StrnCat' with 'StrnCatS' to avoid the buffer > overflow in 'map.c'. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Qiu Shumin <[email protected]> > --- > ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c | 24 > +++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c > index 087daac..16345d3 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Map.c > @@ -2,7 +2,7 @@ > Main file for map shell level 2 command. > > (C) Copyright 2013-2015 Hewlett-Packard Development Company, > L.P.<BR> > - Copyright (c) 2009 - 2014, Intel Corporation. All rights > reserved.<BR> > + Copyright (c) 2009 - 2015, Intel Corporation. All rights > + reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of > the BSD License > which accompanies this distribution. The full text of the license > may be found at @@ -224,6 +224,8 @@ MappingListHasType( > ) > { > CHAR16 *NewSpecific; > + RETURN_STATUS Status; > + > // > // specific has priority > // > @@ -233,7 +235,11 @@ MappingListHasType( > return FALSE; > } > if (NewSpecific[StrLen(NewSpecific)-1] != L':') { > - StrnCat(NewSpecific, L":", 2); > + Status = StrnCatS(NewSpecific, StrSize(Specific) + > + sizeof(CHAR16), L":", > StrLen(L":")); > + if (EFI_ERROR (Status)) { > + FreePool(NewSpecific); > + return FALSE; > + } > } > > if (SearchList(MapList, NewSpecific, NULL, TRUE, FALSE, L";")) { > @@ - > 875,13 +881,18 @@ AddMappingFromMapping( > CONST EFI_DEVICE_PATH_PROTOCOL *DevPath; > EFI_STATUS Status; > CHAR16 *NewSName; > + RETURN_STATUS StrRetStatus; > > NewSName = AllocateCopyPool(StrSize(SName) + sizeof(CHAR16), > SName); > if (NewSName == NULL) { > return (SHELL_OUT_OF_RESOURCES); > } > if (NewSName[StrLen(NewSName)-1] != L':') { > - StrnCat(NewSName, L":", 2); > + StrRetStatus = StrnCatS(NewSName, StrSize(SName) + > + sizeof(CHAR16), > L":", StrLen(L":")); > + if (EFI_ERROR(StrRetStatus)) { > + FreePool(NewSName); > + return ((SHELL_STATUS) (StrRetStatus & (~MAX_BIT))); > + } > } > > if (!IsNumberLetterOnly(NewSName, StrLen(NewSName)-1)) { @@ -927,13 > +938,18 @@ AddMappingFromHandle( > EFI_DEVICE_PATH_PROTOCOL *DevPath; > EFI_STATUS Status; > CHAR16 *NewSName; > + RETURN_STATUS StrRetStatus; > > NewSName = AllocateCopyPool(StrSize(SName) + sizeof(CHAR16), > SName); > if (NewSName == NULL) { > return (SHELL_OUT_OF_RESOURCES); > } > if (NewSName[StrLen(NewSName)-1] != L':') { > - StrnCat(NewSName, L":", 2); > + StrRetStatus = StrnCatS(NewSName, StrSize(SName) + > + sizeof(CHAR16), > L":", StrLen(L":")); > + if (EFI_ERROR(StrRetStatus)) { > + FreePool(NewSName); > + return ((SHELL_STATUS) (StrRetStatus & (~MAX_BIT))); > + } > } > > if (!IsNumberLetterOnly(NewSName, StrLen(NewSName)-1)) { > -- > 1.9.5.msysgit.1 > > > > ---------------------------------------------------------------------- > -------- One dashboard for servers and applications across > Physical-Virtual-Cloud Widest out-of-the-box monitoring support with > 50+ applications Performance metrics, stats and reports that give you > Actionable Insights Deep dive visibility with transaction tracing > using APM Insight. > http://ad.doubleclick.net/ddm/clk/290420510;117567292;y > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > ---------------------------------------------------------------------- > -------- One dashboard for servers and applications across > Physical-Virtual-Cloud Widest out-of-the-box monitoring support with > 50+ applications Performance metrics, stats and reports that give you > Actionable Insights Deep dive visibility with transaction tracing > using APM Insight. > http://ad.doubleclick.net/ddm/clk/290420510;117567292;y > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
