[edk2] [PATCH 0/2] PerformancePkg Dp_App: Use safe string functions

[Hao Wu]

The first patch is to use safe string functions in Dp_App.
The second patch is to resolve a buffer size mismatch problem in
DpUtilities.c.

Details are available in the log message in each commit.

Hao Wu (2):
PerformancePkg Dp_App: Use safe string functions
PerformancePkg Dp_App: Resolve buffer size mismatch

PerformancePkg/Dp_App/DpUtilities.c | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)

[Hao Wu]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Jaben Carsey <***@intel.com>
---
PerformancePkg/Dp_App/DpUtilities.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/PerformancePkg/Dp_App/DpUtilities.c b/PerformancePkg/Dp_App/DpUtilities.c
index 73666aa..38d2293 100644
--- a/PerformancePkg/Dp_App/DpUtilities.c
+++ b/PerformancePkg/Dp_App/DpUtilities.c
@@ -1,7 +1,7 @@
/** @file
Utility functions used by the Dp application.

- 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
@@ -159,7 +159,7 @@ GetShortPdbFileName (
ZeroMem (UnicodeBuffer, DXE_PERFORMANCE_STRING_LENGTH * sizeof (CHAR16));

if (PdbFileName == NULL) {
- StrCpy (UnicodeBuffer, L" ");
+ StrCpyS (UnicodeBuffer, DXE_PERFORMANCE_STRING_SIZE, L" ");
} else {
StartIndex = 0;
for (EndIndex = 0; PdbFileName[EndIndex] != 0; EndIndex++)
@@ -290,8 +290,11 @@ GetNameFromHandle (
);
SafeFreePool (BestLanguage);
if (!EFI_ERROR (Status)) {
- StrnCpy (mGaugeString, StringPtr, DP_GAUGE_STRING_LENGTH);
- mGaugeString[DP_GAUGE_STRING_LENGTH] = 0;
+ StrCpyS (
+ mGaugeString,
+ DP_GAUGE_STRING_LENGTH + 1,
+ StringPtr
+ );
return;
}
}
@@ -334,8 +337,11 @@ GetNameFromHandle (
//
// Method 3. Get the name string from FFS UI section
//
- StrnCpy (mGaugeString, NameString, DP_GAUGE_STRING_LENGTH);
- mGaugeString[DP_GAUGE_STRING_LENGTH] = 0;
+ StrCpyS (
+ mGaugeString,
+ DP_GAUGE_STRING_LENGTH + 1,
+ NameString
+ );
FreePool (NameString);
} else {
//
@@ -350,8 +356,11 @@ GetNameFromHandle (
//
NameString = ConvertDevicePathToText (LoadedImageDevicePath, TRUE, FALSE);
if (NameString != NULL) {
- StrnCpy (mGaugeString, NameString, DP_GAUGE_STRING_LENGTH);
- mGaugeString[DP_GAUGE_STRING_LENGTH] = 0;
+ StrCpyS (
+ mGaugeString,
+ DP_GAUGE_STRING_LENGTH + 1,
+ NameString
+ );
FreePool (NameString);
return;
}
@@ -363,7 +372,7 @@ GetNameFromHandle (
//
StringPtr = HiiGetString (gHiiHandle, STRING_TOKEN (STR_DP_ERROR_NAME), NULL);
ASSERT (StringPtr != NULL);
- StrCpy (mGaugeString, StringPtr);
+ StrCpyS (mGaugeString, DP_GAUGE_STRING_LENGTH + 1, StringPtr);
FreePool (StringPtr);
return;
}

[Hao Wu]

CHAR16 array mGaugeString[DP_GAUGE_STRING_LENGTH + 1] is pass into
function GetShortPdbFileName(). However, in this function it treats the
size of the input buffer as DXE_PERFORMANCE_STRING_SIZE.

Though DXE_PERFORMANCE_STRING_SIZE is smaller than DP_GAUGE_STRING_LENGTH
now, but this manner might introduce a potential risk of buffer overflow.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Star Zeng <***@intel.com>
---
PerformancePkg/Dp_App/DpUtilities.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/PerformancePkg/Dp_App/DpUtilities.c b/PerformancePkg/Dp_App/DpUtilities.c
index 38d2293..38d7aa3 100644
--- a/PerformancePkg/Dp_App/DpUtilities.c
+++ b/PerformancePkg/Dp_App/DpUtilities.c
@@ -156,10 +156,10 @@ GetShortPdbFileName (
UINTN StartIndex;
UINTN EndIndex;

- ZeroMem (UnicodeBuffer, DXE_PERFORMANCE_STRING_LENGTH * sizeof (CHAR16));
+ ZeroMem (UnicodeBuffer, (DP_GAUGE_STRING_LENGTH + 1) * sizeof (CHAR16));

if (PdbFileName == NULL) {
- StrCpyS (UnicodeBuffer, DXE_PERFORMANCE_STRING_SIZE, L" ");
+ StrCpyS (UnicodeBuffer, DP_GAUGE_STRING_LENGTH + 1, L" ");
} else {
StartIndex = 0;
for (EndIndex = 0; PdbFileName[EndIndex] != 0; EndIndex++)
@@ -178,8 +178,8 @@ GetShortPdbFileName (
for (IndexA = StartIndex; IndexA < EndIndex; IndexA++) {
UnicodeBuffer[IndexU] = (CHAR16) PdbFileName[IndexA];
IndexU++;
- if (IndexU >= DXE_PERFORMANCE_STRING_LENGTH) {
- UnicodeBuffer[DXE_PERFORMANCE_STRING_LENGTH] = 0;
+ if (IndexU >= DP_GAUGE_STRING_LENGTH) {
+ UnicodeBuffer[DP_GAUGE_STRING_LENGTH] = 0;
break;
}
}