Discussion:
[edk2] [PATCH 0/8] IntelFrameworkModulePkg: Use safe string functions
Hao Wu
2015-06-23 05:23:46 UTC
Permalink
Replace unsafe string functions:
(Ascii)StrCpy
(Ascii)StrnCpy
(Ascii)StrCat
(Ascii)StrnCat
with new added safe string functions:
(Ascii)StrCpyS
(Ascii)StrnCpyS
(Ascii)StrCatS
(Ascii)StrnCatS
in IntelFrameworkModulePkg.

Hao Wu (8):
IntelFrameworkModulePkg IsaFloppyDxe: Use safe string functions
IntelFrameworkModulePkg GenericBdsLib: Use safe string functions
IntelFrameworkModulePkg PeiDxeDebugLib: Use safe string functions
IntelFrameworkModulePkg BdsDxe: Use safe string functions
IntelFrameworkModulePkg BootMaint: Use safe string functions
IntelFrameworkModulePkg BootMngr: Use safe string functions
IntelFrameworkModulePkg DeviceMngr: Use safe string functions
IntelFrameworkModulePkg UpdateDriverDxe: Use safe string functions

.../Bus/Isa/IsaFloppyDxe/ComponentName.c | 4 +-
.../Library/GenericBdsLib/BdsMisc.c | 6 +-
.../Library/GenericBdsLib/Performance.c | 10 ++--
.../PeiDxeDebugLibReportStatusCode/DebugLib.c | 4 +-
.../Universal/BdsDxe/BootMaint/BootOption.c | 60 +++++++++++++++----
.../Universal/BdsDxe/BootMaint/FormGuid.h | 16 +++--
.../Universal/BdsDxe/BootMaint/UpdatePage.c | 9 ++-
.../Universal/BdsDxe/BootMaint/Variable.c | 10 +++-
.../Universal/BdsDxe/BootMngr/BootManager.c | 12 ++--
.../Universal/BdsDxe/DeviceMngr/DeviceManager.c | 69 ++++++++++++++++++----
.../Universal/BdsDxe/FrontPage.c | 8 +--
.../Universal/BdsDxe/MemoryTest.c | 16 ++++-
.../UpdateDriverDxe/ParseUpdateProfile.c | 6 +-
13 files changed, 168 insertions(+), 62 deletions(-)
--
1.9.5.msysgit.0
Hao Wu
2015-06-23 05:23:47 UTC
Permalink
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/ComponentName.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/ComponentName.c b/IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/ComponentName.c
index a41760e..f3341ed 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/ComponentName.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/ComponentName.c
@@ -1,7 +1,7 @@
/** @file
UEFI Component Name(2) protocol implementation for Isa Floppy driver.

-Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 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
@@ -227,7 +227,7 @@ AddName (
CHAR16 FloppyDriveName[FLOPPY_DRIVE_NAME_LEN + 1];

if (!(FeaturePcdGet(PcdComponentNameDisable) && FeaturePcdGet(PcdComponentName2Disable))) {
- StrCpy (FloppyDriveName, FLOPPY_DRIVE_NAME);
+ StrCpyS (FloppyDriveName, FLOPPY_DRIVE_NAME_LEN + 1, FLOPPY_DRIVE_NAME);
FloppyDriveName[FLOPPY_DRIVE_NAME_LEN - 1] = (CHAR16) (L'0' + FdcDev->Disk);

AddUnicodeString2 (
--
1.9.5.msysgit.0
Hao Wu
2015-06-23 05:23:48 UTC
Permalink
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
IntelFrameworkModulePkg/Library/GenericBdsLib/BdsMisc.c | 6 +++---
IntelFrameworkModulePkg/Library/GenericBdsLib/Performance.c | 10 +++++-----
2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsMisc.c b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsMisc.c
index dbb1322..951dc9d 100644
--- a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsMisc.c
+++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsMisc.c
@@ -1,7 +1,7 @@
/** @file
Misc BDS library function

-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -1131,8 +1131,8 @@ SetupResetReminder (
ASSERT (StringBuffer1 != NULL);
StringBuffer2 = AllocateZeroPool (MAX_STRING_LEN * sizeof (CHAR16));
ASSERT (StringBuffer2 != NULL);
- StrCpy (StringBuffer1, L"Configuration changed. Reset to apply it Now.");
- StrCpy (StringBuffer2, L"Press ENTER to reset");
+ StrCpyS (StringBuffer1, MAX_STRING_LEN, L"Configuration changed. Reset to apply it Now.");
+ StrCpyS (StringBuffer2, MAX_STRING_LEN, L"Press ENTER to reset");
//
// Popup a menu to notice user
//
diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/Performance.c b/IntelFrameworkModulePkg/Library/GenericBdsLib/Performance.c
index 047d2a7..000542b 100644
--- a/IntelFrameworkModulePkg/Library/GenericBdsLib/Performance.c
+++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/Performance.c
@@ -3,7 +3,7 @@
performance, all the function will only include if the performance
switch is set.

-Copyright (c) 2004 - 2013, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -40,7 +40,7 @@ GetShortPdbFileName (
UINTN EndIndex;

if (PdbFileName == NULL) {
- AsciiStrCpy (GaugeString, " ");
+ AsciiStrCpyS (GaugeString, PERF_TOKEN_LENGTH, " ");
} else {
StartIndex = 0;
for (EndIndex = 0; PdbFileName[EndIndex] != 0; EndIndex++)
@@ -91,7 +91,7 @@ GetNameFromHandle (
CHAR8 *PdbFileName;
EFI_DRIVER_BINDING_PROTOCOL *DriverBinding;

- AsciiStrCpy (GaugeString, " ");
+ AsciiStrCpyS (GaugeString, PERF_TOKEN_LENGTH, " ");

//
// Get handle name from image protocol
@@ -287,7 +287,7 @@ WriteBootToOsPerformanceData (

GetNameFromHandle (Handles[Index], GaugeString);

- AsciiStrCpy (mPerfData.Token, GaugeString);
+ AsciiStrCpyS (mPerfData.Token, PERF_TOKEN_SIZE, GaugeString);
mPerfData.Duration = Duration;

CopyMem (Ptr, &mPerfData, sizeof (PERF_DATA));
@@ -316,7 +316,7 @@ WriteBootToOsPerformanceData (

ZeroMem (&mPerfData, sizeof (PERF_DATA));

- AsciiStrnCpy (mPerfData.Token, Token, PERF_TOKEN_LENGTH);
+ AsciiStrnCpyS (mPerfData.Token, PERF_TOKEN_SIZE, Token, PERF_TOKEN_LENGTH);
if (StartTicker == 1) {
StartTicker = StartValue;
}
--
1.9.5.msysgit.0
Hao Wu
2015-06-23 05:23:49 UTC
Permalink
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
.../Library/PeiDxeDebugLibReportStatusCode/DebugLib.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c b/IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
index 68c1a55..2d39b4a 100644
--- a/IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
+++ b/IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
@@ -56,6 +56,7 @@ DebugPrint (
UINT64 Buffer[(EFI_STATUS_CODE_DATA_MAX_SIZE / sizeof (UINT64)) + 1];
EFI_DEBUG_INFO *DebugInfo;
UINTN TotalSize;
+ UINTN DestBufferSize;
VA_LIST VaListMarker;
BASE_LIST BaseListMarker;
CHAR8 *FormatString;
@@ -115,7 +116,8 @@ DebugPrint (
//
// Copy the Format string into the record
//
- AsciiStrCpy (FormatString, Format);
+ DestBufferSize = sizeof (Buffer) - 4 - sizeof (EFI_DEBUG_INFO) - 12 * sizeof (UINT64);
+ AsciiStrCpyS (FormatString, DestBufferSize / sizeof (CHAR8), Format);

//
// The first 12 * sizeof (UINT64) bytes following EFI_DEBUG_INFO are for variable arguments
--
1.9.5.msysgit.0
Hao Wu
2015-06-23 05:23:50 UTC
Permalink
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 8 ++++----
IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c | 16 +++++++++++++---
2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
index a0c6381..5646457 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
@@ -1,7 +1,7 @@
/** @file
FrontPage routines to handle the callbacks and browser calls

-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -628,9 +628,9 @@ ConvertProcessorToString (
StringBuffer = AllocateZeroPool (0x20);
ASSERT (StringBuffer != NULL);
Index = UnicodeValueToString (StringBuffer, LEFT_JUSTIFY, FreqMhz / 1000, 3);
- StrCat (StringBuffer, L".");
+ StrCatS (StringBuffer, 0x20 / sizeof (CHAR16), L".");
UnicodeValueToString (StringBuffer + Index + 1, PREFIX_ZERO, (FreqMhz % 1000) / 10, 2);
- StrCat (StringBuffer, L" GHz");
+ StrCatS (StringBuffer, 0x20 / sizeof (CHAR16), L" GHz");
*String = (CHAR16 *) StringBuffer;
return ;
}
@@ -654,7 +654,7 @@ ConvertMemorySizeToString (
StringBuffer = AllocateZeroPool (0x20);
ASSERT (StringBuffer != NULL);
UnicodeValueToString (StringBuffer, LEFT_JUSTIFY, MemorySize, 6);
- StrCat (StringBuffer, L" MB RAM");
+ StrCatS (StringBuffer, 0x20 / sizeof (CHAR16), L" MB RAM");

*String = (CHAR16 *) StringBuffer;

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c b/IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c
index 5a6fa78..2857c82 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c
@@ -1,7 +1,7 @@
/** @file
Perform the platform memory test

-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -324,7 +324,12 @@ BdsMemoryTest (
//
// TmpStr size is 64, StrPercent is reserved to 16.
//
- StrnCat (StrPercent, TmpStr, sizeof (StrPercent) / sizeof (CHAR16) - StrLen (StrPercent) - 1);
+ StrnCatS (
+ StrPercent,
+ sizeof (StrPercent) / sizeof (CHAR16),
+ TmpStr,
+ sizeof (StrPercent) / sizeof (CHAR16) - StrLen (StrPercent) - 1
+ );
PrintXY (10, 10, NULL, NULL, StrPercent);
FreePool (TmpStr);
}
@@ -389,7 +394,12 @@ Done:

TmpStr = GetStringById (STRING_TOKEN (STR_MEM_TEST_COMPLETED));
if (TmpStr != NULL) {
- StrnCat (StrTotalMemory, TmpStr, StrTotalMemorySize / sizeof (CHAR16) - StrLen (StrTotalMemory) - 1);
+ StrnCatS (
+ StrTotalMemory,
+ StrTotalMemorySize / sizeof (CHAR16),
+ TmpStr,
+ StrTotalMemorySize / sizeof (CHAR16) - StrLen (StrTotalMemory) - 1
+ );
FreePool (TmpStr);
}
--
1.9.5.msysgit.0
Hao Wu
2015-06-23 05:23:51 UTC
Permalink
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
.../Universal/BdsDxe/BootMaint/BootOption.c | 60 +++++++++++++++++-----
.../Universal/BdsDxe/BootMaint/FormGuid.h | 16 ++++--
.../Universal/BdsDxe/BootMaint/UpdatePage.c | 9 +++-
.../Universal/BdsDxe/BootMaint/Variable.c | 10 ++--
4 files changed, 72 insertions(+), 23 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
index 0a6a445..a55dd89 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
@@ -5,7 +5,7 @@

Boot option manipulation

-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -1012,7 +1012,11 @@ BOpt_GetBootOptions (

NewLoadContext->Description = AllocateZeroPool (StrSize((UINT16*)LoadOptionPtr));
ASSERT (NewLoadContext->Description != NULL);
- StrCpy (NewLoadContext->Description, (UINT16*)LoadOptionPtr);
+ StrCpyS (
+ NewLoadContext->Description,
+ StrSize((UINT16*)LoadOptionPtr) / sizeof (UINT16),
+ (UINT16*)LoadOptionPtr
+ );

ASSERT (NewLoadContext->Description != NULL);
NewMenuEntry->DisplayString = NewLoadContext->Description;
@@ -1089,6 +1093,7 @@ BOpt_AppendFileName (
{
UINTN Size1;
UINTN Size2;
+ UINTN MaxLen;
CHAR16 *Str;
CHAR16 *TmpStr;
CHAR16 *Ptr;
@@ -1096,18 +1101,31 @@ BOpt_AppendFileName (

Size1 = StrSize (Str1);
Size2 = StrSize (Str2);
- Str = AllocateZeroPool (Size1 + Size2 + sizeof (CHAR16));
+ MaxLen = (Size1 + Size2 + sizeof (CHAR16)) / sizeof (CHAR16);
+ Str = AllocateZeroPool (MaxLen * sizeof (CHAR16));
ASSERT (Str != NULL);

- TmpStr = AllocateZeroPool (Size1 + Size2 + sizeof (CHAR16));
+ TmpStr = AllocateZeroPool (MaxLen * sizeof (CHAR16));
ASSERT (TmpStr != NULL);

- StrCat (Str, Str1);
+ StrCatS (
+ Str,
+ MaxLen,
+ Str1
+ );
if (!((*Str == '\\') && (*(Str + 1) == 0))) {
- StrCat (Str, L"\\");
+ StrCatS (
+ Str,
+ MaxLen,
+ L"\\"
+ );
}

- StrCat (Str, Str2);
+ StrCatS (
+ Str,
+ MaxLen,
+ Str2
+ );

Ptr = Str;
LastSlash = Str;
@@ -1120,11 +1138,19 @@ BOpt_AppendFileName (
//

//
- // Use TmpStr as a backup, as StrCpy in BaseLib does not handle copy of two strings
+ // Use TmpStr as a backup, as StrCpyS in BaseLib does not handle copy of two strings
// that overlap.
//
- StrCpy (TmpStr, Ptr + 3);
- StrCpy (LastSlash, TmpStr);
+ StrCpyS (
+ TmpStr,
+ MaxLen,
+ Ptr + 3
+ );
+ StrCpyS (
+ LastSlash,
+ MaxLen - (UINTN) (LastSlash - Str) / sizeof (CHAR16),
+ TmpStr
+ );
Ptr = LastSlash;
} else if (*Ptr == '\\' && *(Ptr + 1) == '.' && *(Ptr + 2) == '\\') {
//
@@ -1132,11 +1158,19 @@ BOpt_AppendFileName (
//

//
- // Use TmpStr as a backup, as StrCpy in BaseLib does not handle copy of two strings
+ // Use TmpStr as a backup, as StrCpyS in BaseLib does not handle copy of two strings
// that overlap.
//
- StrCpy (TmpStr, Ptr + 2);
- StrCpy (Ptr, TmpStr);
+ StrCpyS (
+ TmpStr,
+ MaxLen,
+ Ptr + 2
+ );
+ StrCpyS (
+ Ptr,
+ MaxLen - (UINTN) (Ptr - Str) / sizeof (CHAR16),
+ TmpStr
+ );
Ptr = LastSlash;
} else if (*Ptr == '\\') {
LastSlash = Ptr;
diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h
index f2e1866..bf99999 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h
@@ -1,7 +1,7 @@
/** @file
Formset guids, form id and VarStore data structure for Boot Maintenance Manager.

-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -219,14 +219,20 @@ typedef struct {
#define KEY_VALUE_SAVE_AND_EXIT_DRIVER 0x1002
#define KEY_VALUE_NO_SAVE_AND_EXIT_DRIVER 0x1003

+//
+// Description data and optional data size
+//
+#define DESCRIPTION_DATA_SIZE 75
+#define OPTIONAL_DATA_SIZE 127
+
///
/// This is the data structure used by File Explorer formset
///
typedef struct {
- UINT16 BootDescriptionData[75];
- UINT16 BootOptionalData[127];
- UINT16 DriverDescriptionData[75];
- UINT16 DriverOptionalData[127];
+ UINT16 BootDescriptionData[DESCRIPTION_DATA_SIZE];
+ UINT16 BootOptionalData[OPTIONAL_DATA_SIZE];
+ UINT16 DriverDescriptionData[DESCRIPTION_DATA_SIZE];
+ UINT16 DriverOptionalData[OPTIONAL_DATA_SIZE];
BOOLEAN BootOptionChanged;
BOOLEAN DriverOptionChanged;
UINT8 Active;
diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
index 7d5861e..78380ad 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
@@ -1,7 +1,7 @@
/** @file
Dynamically update the pages.

-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -830,7 +830,12 @@ UpdateConModePage (
//
UnicodeValueToString (ModeString, 0, Col, 0);
PStr = &ModeString[0];
- StrnCat (PStr, L" x ", StrLen(L" x ") + 1);
+ StrnCatS (
+ PStr,
+ sizeof (ModeString) / sizeof (ModeString[0]),
+ L" x ",
+ sizeof (ModeString) / sizeof (ModeString[0]) - StrLen (PStr) - 1
+ );
PStr = PStr + StrLen (PStr);
UnicodeValueToString (PStr , 0, Row, 0);

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c
index e4299ff..616549e 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c
@@ -1,7 +1,7 @@
/** @file
Variable operation that will be used by bootmaint

-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -579,7 +579,7 @@ Var_UpdateDriverOption (
);

if (*DescriptionData == 0x0000) {
- StrCpy (DescriptionData, DriverString);
+ StrCpyS (DescriptionData, DESCRIPTION_DATA_SIZE, DriverString);
}

BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (DescriptionData);
@@ -763,7 +763,11 @@ Var_UpdateBootOption (
UnicodeSPrint (BootString, sizeof (BootString), L"Boot%04x", Index);

if (NvRamMap->BootDescriptionData[0] == 0x0000) {
- StrCpy (NvRamMap->BootDescriptionData, BootString);
+ StrCpyS (
+ NvRamMap->BootDescriptionData,
+ sizeof (NvRamMap->BootDescriptionData) / sizeof (NvRamMap->BootDescriptionData[0]),
+ BootString
+ );
}

BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (NvRamMap->BootDescriptionData);
--
1.9.5.msysgit.0
Scott Duplichan
2015-06-23 15:54:44 UTC
Permalink
Hao Wu [mailto:***@intel.com] wrote:

]Sent: Tuesday, June 23, 2015 12:24 AM
]To: edk2-***@lists.sourceforge.net
]Subject: [edk2] [PATCH 5/8] IntelFrameworkModulePkg BootMaint: Use safe string functions
]
]Contributed-under: TianoCore Contribution Agreement 1.0
]Signed-off-by: Hao Wu <***@intel.com>
]Reviewed-by: Jeff Fan <***@intel.com>
]---
] .../Universal/BdsDxe/BootMaint/BootOption.c | 60 +++++++++++++++++-----
] .../Universal/BdsDxe/BootMaint/FormGuid.h | 16 ++++--
] .../Universal/BdsDxe/BootMaint/UpdatePage.c | 9 +++-
] .../Universal/BdsDxe/BootMaint/Variable.c | 10 ++--
] 4 files changed, 72 insertions(+), 23 deletions(-)
]
]diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
]index 0a6a445..a55dd89 100644
]--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
]+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
]@@ -5,7 +5,7 @@
]
] Boot option manipulation
]
]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
]+Copyright (c) 2004 - 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
]@@ -1012,7 +1012,11 @@ BOpt_GetBootOptions (
]
] NewLoadContext->Description = AllocateZeroPool (StrSize((UINT16*)LoadOptionPtr));
] ASSERT (NewLoadContext->Description != NULL);
]- StrCpy (NewLoadContext->Description, (UINT16*)LoadOptionPtr);
]+ StrCpyS (
]+ NewLoadContext->Description,
]+ StrSize((UINT16*)LoadOptionPtr) / sizeof (UINT16),
]+ (UINT16*)LoadOptionPtr
]+ );
]
] ASSERT (NewLoadContext->Description != NULL);
] NewMenuEntry->DisplayString = NewLoadContext->Description;
]@@ -1089,6 +1093,7 @@ BOpt_AppendFileName (
] {
] UINTN Size1;
] UINTN Size2;
]+ UINTN MaxLen;
] CHAR16 *Str;
] CHAR16 *TmpStr;
] CHAR16 *Ptr;
]@@ -1096,18 +1101,31 @@ BOpt_AppendFileName (
]
] Size1 = StrSize (Str1);
] Size2 = StrSize (Str2);
]- Str = AllocateZeroPool (Size1 + Size2 + sizeof (CHAR16));
]+ MaxLen = (Size1 + Size2 + sizeof (CHAR16)) / sizeof (CHAR16);
]+ Str = AllocateZeroPool (MaxLen * sizeof (CHAR16));
] ASSERT (Str != NULL);
]
]- TmpStr = AllocateZeroPool (Size1 + Size2 + sizeof (CHAR16));
]+ TmpStr = AllocateZeroPool (MaxLen * sizeof (CHAR16));
] ASSERT (TmpStr != NULL);
]
]- StrCat (Str, Str1);
]+ StrCatS (
]+ Str,
]+ MaxLen,
]+ Str1
]+ );
] if (!((*Str == '\\') && (*(Str + 1) == 0))) {
]- StrCat (Str, L"\\");
]+ StrCatS (
]+ Str,
]+ MaxLen,
]+ L"\\"
]+ );
] }
]
]- StrCat (Str, Str2);
]+ StrCatS (
]+ Str,
]+ MaxLen,
]+ Str2
]+ );
]
] Ptr = Str;
] LastSlash = Str;
]@@ -1120,11 +1138,19 @@ BOpt_AppendFileName (
] //
]
] //
]- // Use TmpStr as a backup, as StrCpy in BaseLib does not handle copy of two strings
]+ // Use TmpStr as a backup, as StrCpyS in BaseLib does not handle copy of two strings
] // that overlap.
] //
]- StrCpy (TmpStr, Ptr + 3);
]- StrCpy (LastSlash, TmpStr);
]+ StrCpyS (
]+ TmpStr,
]+ MaxLen,
]+ Ptr + 3
]+ );
]+ StrCpyS (
]+ LastSlash,
]+ MaxLen - (UINTN) (LastSlash - Str) / sizeof (CHAR16),

Does "/ sizeof (CHAR16)" belong here? I think the subtraction of
two CHAR16 pointers already gives a result in units of CHAR16.

]+ TmpStr
]+ );
] Ptr = LastSlash;
] } else if (*Ptr == '\\' && *(Ptr + 1) == '.' && *(Ptr + 2) == '\\') {
] //
]@@ -1132,11 +1158,19 @@ BOpt_AppendFileName (
] //
]
] //
]- // Use TmpStr as a backup, as StrCpy in BaseLib does not handle copy of two strings
]+ // Use TmpStr as a backup, as StrCpyS in BaseLib does not handle copy of two strings
] // that overlap.
] //
]- StrCpy (TmpStr, Ptr + 2);
]- StrCpy (Ptr, TmpStr);
]+ StrCpyS (
]+ TmpStr,
]+ MaxLen,
]+ Ptr + 2
]+ );
]+ StrCpyS (
]+ Ptr,
]+ MaxLen - (UINTN) (Ptr - Str) / sizeof (CHAR16),

Does "/ sizeof (CHAR16)" belong here? I think the subtraction of
two CHAR16 pointers already gives a result in units of CHAR16.

]+ TmpStr
]+ );
] Ptr = LastSlash;
] } else if (*Ptr == '\\') {
] LastSlash = Ptr;
]diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h
]index f2e1866..bf99999 100644
]--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h
]+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h
]@@ -1,7 +1,7 @@
] /** @file
] Formset guids, form id and VarStore data structure for Boot Maintenance Manager.
]
]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
]+Copyright (c) 2004 - 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
]@@ -219,14 +219,20 @@ typedef struct {
] #define KEY_VALUE_SAVE_AND_EXIT_DRIVER 0x1002
] #define KEY_VALUE_NO_SAVE_AND_EXIT_DRIVER 0x1003
]
]+//
]+// Description data and optional data size
]+//
]+#define DESCRIPTION_DATA_SIZE 75
]+#define OPTIONAL_DATA_SIZE 127
]+
] ///
] /// This is the data structure used by File Explorer formset
] ///
] typedef struct {
]- UINT16 BootDescriptionData[75];
]- UINT16 BootOptionalData[127];
]- UINT16 DriverDescriptionData[75];
]- UINT16 DriverOptionalData[127];
]+ UINT16 BootDescriptionData[DESCRIPTION_DATA_SIZE];
]+ UINT16 BootOptionalData[OPTIONAL_DATA_SIZE];
]+ UINT16 DriverDescriptionData[DESCRIPTION_DATA_SIZE];
]+ UINT16 DriverOptionalData[OPTIONAL_DATA_SIZE];
] BOOLEAN BootOptionChanged;
] BOOLEAN DriverOptionChanged;
] UINT8 Active;
]diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
]index 7d5861e..78380ad 100644
]--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
]+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
]@@ -1,7 +1,7 @@
] /** @file
] Dynamically update the pages.
]
]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
]+Copyright (c) 2004 - 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
]@@ -830,7 +830,12 @@ UpdateConModePage (
] //
] UnicodeValueToString (ModeString, 0, Col, 0);
] PStr = &ModeString[0];
]- StrnCat (PStr, L" x ", StrLen(L" x ") + 1);
]+ StrnCatS (
]+ PStr,
]+ sizeof (ModeString) / sizeof (ModeString[0]),
]+ L" x ",
]+ sizeof (ModeString) / sizeof (ModeString[0]) - StrLen (PStr) - 1
]+ );
] PStr = PStr + StrLen (PStr);
] UnicodeValueToString (PStr , 0, Row, 0);
]
]diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c
]index e4299ff..616549e 100644
]--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c
]+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c
]@@ -1,7 +1,7 @@
] /** @file
] Variable operation that will be used by bootmaint
]
]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
]+Copyright (c) 2004 - 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
]@@ -579,7 +579,7 @@ Var_UpdateDriverOption (
] );
]
] if (*DescriptionData == 0x0000) {
]- StrCpy (DescriptionData, DriverString);
]+ StrCpyS (DescriptionData, DESCRIPTION_DATA_SIZE, DriverString);
] }
]
] BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (DescriptionData);
]@@ -763,7 +763,11 @@ Var_UpdateBootOption (
] UnicodeSPrint (BootString, sizeof (BootString), L"Boot%04x", Index);
]
] if (NvRamMap->BootDescriptionData[0] == 0x0000) {
]- StrCpy (NvRamMap->BootDescriptionData, BootString);
]+ StrCpyS (
]+ NvRamMap->BootDescriptionData,
]+ sizeof (NvRamMap->BootDescriptionData) / sizeof (NvRamMap->BootDescriptionData[0]),
]+ BootString
]+ );
] }
]
] BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (NvRamMap->BootDescriptionData);
]--
]1.9.5.msysgit.0

Hao Wu
2015-06-23 05:23:54 UTC
Permalink
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
.../Universal/FirmwareVolume/UpdateDriverDxe/ParseUpdateProfile.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/FirmwareVolume/UpdateDriverDxe/ParseUpdateProfile.c b/IntelFrameworkModulePkg/Universal/FirmwareVolume/UpdateDriverDxe/ParseUpdateProfile.c
index 17e728d..8c3ee5c 100644
--- a/IntelFrameworkModulePkg/Universal/FirmwareVolume/UpdateDriverDxe/ParseUpdateProfile.c
+++ b/IntelFrameworkModulePkg/Universal/FirmwareVolume/UpdateDriverDxe/ParseUpdateProfile.c
@@ -3,7 +3,7 @@
configuration file and pass the information to the update driver
so that the driver can perform updates accordingly.

- Copyright (c) 2002 - 2010, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2002 - 2015, Intel Corporation. All rights reserved.<BR>

This program and the accompanying materials
are licensed and made available under the terms and conditions
@@ -678,7 +678,7 @@ UpdateStringToGuid (
if (Buffer == NULL) {
return EFI_OUT_OF_RESOURCES;
}
- AsciiStrCpy ((CHAR8 *)Buffer, (CHAR8 *)Str);
+ AsciiStrCpyS ((CHAR8 *)Buffer, (StrLen + 1) / sizeof (UINT8), (CHAR8 *)Str);

//
// Data1
@@ -997,7 +997,7 @@ ParseUpdateDataFile (
//
// Get the section name of each update
//
- AsciiStrCpy (Entry, "Update");
+ AsciiStrCpyS (Entry, MAX_LINE_LENGTH, "Update");
UpdateStrCatNumber ((UINT8 *) Entry, Index);
Value = NULL;
Status = UpdateGetProfileString (
--
1.9.5.msysgit.0
Hao Wu
2015-06-23 05:23:52 UTC
Permalink
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
.../Universal/BdsDxe/BootMngr/BootManager.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMngr/BootManager.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMngr/BootManager.c
index dc13648..6efd783 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMngr/BootManager.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMngr/BootManager.c
@@ -1,7 +1,7 @@
/** @file
The platform boot manager reference implementation

-Copyright (c) 2004 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -205,11 +205,11 @@ CallBootManager (
EFI_STRING_ID Token;
EFI_INPUT_KEY Key;
CHAR16 *HelpString;
+ UINTN HelpSize;
EFI_STRING_ID HelpToken;
UINT16 *TempStr;
EFI_HII_HANDLE HiiHandle;
EFI_BROWSER_ACTION_REQUEST ActionRequest;
- UINTN TempSize;
VOID *StartOpCodeHandle;
VOID *EndOpCodeHandle;
EFI_IFR_GUID_LABEL *StartLabel;
@@ -318,11 +318,11 @@ CallBootManager (
Token = HiiSetString (HiiHandle, 0, Option->Description, NULL);

TempStr = DevicePathToStr (Option->DevicePath);
- TempSize = StrSize (TempStr);
- HelpString = AllocateZeroPool (TempSize + StrSize (L"Device Path : "));
+ HelpSize = StrSize (TempStr) + StrSize (L"Device Path : ");
+ HelpString = AllocateZeroPool (HelpSize);
ASSERT (HelpString != NULL);
- StrCat (HelpString, L"Device Path : ");
- StrCat (HelpString, TempStr);
+ StrCatS (HelpString, HelpSize / sizeof (CHAR16), L"Device Path : ");
+ StrCatS (HelpString, HelpSize / sizeof (CHAR16), TempStr);

HelpToken = HiiSetString (HiiHandle, 0, HelpString, NULL);
--
1.9.5.msysgit.0
Hao Wu
2015-06-23 05:23:53 UTC
Permalink
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
.../Universal/BdsDxe/DeviceMngr/DeviceManager.c | 69 ++++++++++++++++++----
1 file changed, 57 insertions(+), 12 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c b/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c
index 85cbe89..48182b8 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/DeviceMngr/DeviceManager.c
@@ -1,7 +1,7 @@
/** @file
The platform device manager reference implementation

-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
@@ -380,7 +380,7 @@ GetMacAddressString(
}

*PBuffer = String;
- StrCpy(String, L"MAC:");
+ StrCpyS (String, BufferLen / sizeof (CHAR16), L"MAC:");
String += 4;

//
@@ -1383,15 +1383,40 @@ CallDriverHealth (
String = (EFI_STRING) AllocateZeroPool (StringSize);
ASSERT (String != NULL);

- StrnCpy (String, DriverName, StringSize / sizeof(CHAR16));
+ StrnCpyS (
+ String,
+ StringSize / sizeof(CHAR16),
+ DriverName,
+ StringSize / sizeof(CHAR16) - 1
+ );
if (!IsControllerNameEmpty) {
- StrnCat (String, L" ", StringSize / sizeof(CHAR16) - StrLen(String) - 1);
- StrnCat (String, ControllerName, StringSize / sizeof(CHAR16) - StrLen(String) - 1);
+ StrnCatS (
+ String,
+ StringSize / sizeof(CHAR16),
+ L" ",
+ StringSize / sizeof(CHAR16) - StrLen(String) - 1
+ );
+ StrnCatS (
+ String,
+ StringSize / sizeof(CHAR16),
+ ControllerName,
+ StringSize / sizeof(CHAR16) - StrLen(String) - 1
+ );
}

- StrnCat (String, L" ", StringSize / sizeof(CHAR16) - StrLen(String) - 1);
- StrnCat (String, TmpString, StringSize / sizeof(CHAR16) - StrLen(String) - 1);
-
+ StrnCatS (
+ String,
+ StringSize / sizeof(CHAR16),
+ L" ",
+ StringSize / sizeof(CHAR16) - StrLen(String) - 1
+ );
+ StrnCatS (
+ String,
+ StringSize / sizeof(CHAR16),
+ TmpString,
+ StringSize / sizeof(CHAR16) - StrLen(String) - 1
+ );
+
} else {
//
// Update the string will be displayed base on the driver's health status
@@ -1423,13 +1448,33 @@ CallDriverHealth (
String = (EFI_STRING) AllocateZeroPool (StringSize);
ASSERT (String != NULL);

- StrnCpy (String, DriverName, StringSize / sizeof(CHAR16));
+ StrnCpyS (
+ String,
+ StringSize / sizeof (CHAR16),
+ DriverName,
+ StringSize / sizeof (CHAR16) - 1
+ );
if (!IsControllerNameEmpty) {
- StrnCat (String, L" ", StringSize / sizeof(CHAR16) - StrLen(String) - 1);
- StrnCat (String, ControllerName, StringSize / sizeof(CHAR16) - StrLen(String) - 1);
+ StrnCatS (
+ String,
+ StringSize / sizeof (CHAR16),
+ L" ",
+ StringSize / sizeof(CHAR16) - StrLen(String) - 1
+ );
+ StrnCatS (
+ String,
+ StringSize / sizeof (CHAR16),
+ ControllerName,
+ StringSize / sizeof(CHAR16) - StrLen(String) - 1
+ );
}

- StrnCat (String, TmpString, StringSize / sizeof(CHAR16) - StrLen(String) - 1);
+ StrnCatS (
+ String,
+ StringSize / sizeof (CHAR16),
+ TmpString,
+ StringSize / sizeof(CHAR16) - StrLen(String) - 1
+ );
}

FreePool (TmpString);
--
1.9.5.msysgit.0
Loading...