Discussion:
[edk2] [PATCH] FatPkg EnhancedFatDxe: Use safe string functions
Hao Wu
2015-07-01 08:00:05 UTC
Permalink
Unsafe string functions are replaced with safe ones.

Safe string functions will assert if DestMax is not greater than
StrnLenS(Source, DestMax). Therefore, additional assert for checking the
size of source and destination buffers can be removed.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Ruiyu Ni <***@intel.com>
---
EnhancedFatDxe/DirectoryManage.c | 18 +++++++++++++++---
EnhancedFatDxe/Fat.h | 5 +++--
EnhancedFatDxe/FileName.c | 14 +++++++-------
EnhancedFatDxe/Hash.c | 9 ++++++---
4 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/EnhancedFatDxe/DirectoryManage.c b/EnhancedFatDxe/DirectoryManage.c
index 53e80f7..4b44ec3 100644
--- a/EnhancedFatDxe/DirectoryManage.c
+++ b/EnhancedFatDxe/DirectoryManage.c
@@ -1,6 +1,6 @@
/*++

-Copyright (c) 2005 - 2013, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 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 Software
License Agreement which accompanies this distribution.
@@ -112,7 +112,15 @@ Returns:
// Write LFN directory entry
//
SetMem (LfnBuffer, sizeof (CHAR16) * LFN_CHAR_TOTAL * EntryCount, 0xff);
- StrCpy (LfnBuffer, DirEnt->FileString);
+ Status = StrCpyS (
+ LfnBuffer,
+ sizeof (LfnBuffer) / sizeof (LfnBuffer[0]),
+ DirEnt->FileString
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
LfnBufferPointer = LfnBuffer;
LfnEntry.Attributes = FAT_ATTRIBUTE_LFN;
LfnEntry.Type = 0;
@@ -345,7 +353,11 @@ Returns:
// Fail to get the long file name from long file name entry,
// get the file name from short name
//
- FatGetFileNameViaCaseFlag (DirEnt, LfnBuffer);
+ FatGetFileNameViaCaseFlag (
+ DirEnt,
+ LfnBuffer,
+ sizeof (LfnBuffer) / sizeof (LfnBuffer[0])
+ );
}

DirEnt->FileString = AllocateCopyPool (StrSize (LfnBuffer), LfnBuffer);
diff --git a/EnhancedFatDxe/Fat.h b/EnhancedFatDxe/Fat.h
index 7a3cd06..27d7937 100644
--- a/EnhancedFatDxe/Fat.h
+++ b/EnhancedFatDxe/Fat.h
@@ -1237,8 +1237,9 @@ FatSetCaseFlag (

VOID
FatGetFileNameViaCaseFlag (
- IN FAT_DIRENT *DirEnt,
- OUT CHAR16 *FileString
+ IN FAT_DIRENT *DirEnt,
+ IN OUT CHAR16 *FileString,
+ IN UINTN FileStringMax
);

UINT8
diff --git a/EnhancedFatDxe/FileName.c b/EnhancedFatDxe/FileName.c
index 1ba2706..1f8aad2 100644
--- a/EnhancedFatDxe/FileName.c
+++ b/EnhancedFatDxe/FileName.c
@@ -1,6 +1,6 @@
/*++

-Copyright (c) 2005 - 2007, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 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 Software
License Agreement which accompanies this distribution.
@@ -298,7 +298,6 @@ Returns:
CHAR16 Buffer[FAT_MAIN_NAME_LEN + 1 + FAT_EXTEND_NAME_LEN + 1];
UINT8 OutCaseFlag;

- ASSERT (StrSize (Str) <= sizeof (Buffer));
//
// Assume the case of input string is mixed
//
@@ -307,7 +306,7 @@ Returns:
// Lower case a copy of the string, if it matches the
// original then the string is lower case
//
- StrCpy (Buffer, Str);
+ StrCpyS (Buffer, sizeof (Buffer) / sizeof (Buffer[0]), Str);
FatStrLwr (Buffer);
if (StrCmp (Str, Buffer) == 0) {
OutCaseFlag = InCaseFlag;
@@ -316,7 +315,7 @@ Returns:
// Upper case a copy of the string, if it matches the
// original then the string is upper case
//
- StrCpy (Buffer, Str);
+ StrCpyS (Buffer, sizeof (Buffer) / sizeof (Buffer[0]), Str);
FatStrUpr (Buffer);
if (StrCmp (Str, Buffer) == 0) {
OutCaseFlag = 0;
@@ -388,8 +387,9 @@ Returns:

VOID
FatGetFileNameViaCaseFlag (
- IN FAT_DIRENT *DirEnt,
- OUT CHAR16 *FileString
+ IN FAT_DIRENT *DirEnt,
+ IN OUT CHAR16 *FileString,
+ IN UINTN FileStringMax
)
/*++

@@ -421,7 +421,7 @@ Returns:
FatNameToStr (File8Dot3Name + FAT_MAIN_NAME_LEN, FAT_EXTEND_NAME_LEN, CaseFlag & FAT_CASE_EXT_LOWER, &TempExt[1]);
if (TempExt[1] != 0) {
TempExt[0] = L'.';
- StrCat (FileString, TempExt);
+ StrCatS (FileString, FileStringMax, TempExt);
}
}

diff --git a/EnhancedFatDxe/Hash.c b/EnhancedFatDxe/Hash.c
index d186e35..d933f90 100644
--- a/EnhancedFatDxe/Hash.c
+++ b/EnhancedFatDxe/Hash.c
@@ -1,6 +1,6 @@
/*++

-Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 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 Software
License Agreement which accompanies this distribution.
@@ -43,8 +43,11 @@ Returns:
{
UINT32 HashValue;
CHAR16 UpCasedLongFileName[EFI_PATH_STRING_LENGTH];
- StrnCpy (UpCasedLongFileName, LongNameString, EFI_PATH_STRING_LENGTH - 1);
- UpCasedLongFileName[EFI_PATH_STRING_LENGTH - 1] = L'\0';
+ StrCpyS (
+ UpCasedLongFileName,
+ sizeof (UpCasedLongFileName) / sizeof (UpCasedLongFileName[0]),
+ LongNameString
+ );
FatStrUpr (UpCasedLongFileName);
gBS->CalculateCrc32 (UpCasedLongFileName, StrSize (UpCasedLongFileName), &HashValue);
return (HashValue & HASH_TABLE_MASK);
--
1.9.5.msysgit.0
Carsey, Jaben
2015-07-01 20:30:34 UTC
Permalink
One question below.
-----Original Message-----
Sent: Wednesday, July 01, 2015 1:00 AM
Subject: [edk2] [PATCH] FatPkg EnhancedFatDxe: Use safe string functions
Unsafe string functions are replaced with safe ones.
Safe string functions will assert if DestMax is not greater than
StrnLenS(Source, DestMax). Therefore, additional assert for checking the
size of source and destination buffers can be removed.
Contributed-under: TianoCore Contribution Agreement 1.0
---
EnhancedFatDxe/DirectoryManage.c | 18 +++++++++++++++---
EnhancedFatDxe/Fat.h | 5 +++--
EnhancedFatDxe/FileName.c | 14 +++++++-------
EnhancedFatDxe/Hash.c | 9 ++++++---
4 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/EnhancedFatDxe/DirectoryManage.c
b/EnhancedFatDxe/DirectoryManage.c
index 53e80f7..4b44ec3 100644
--- a/EnhancedFatDxe/DirectoryManage.c
+++ b/EnhancedFatDxe/DirectoryManage.c
@@ -1,6 +1,6 @@
/*++
-Copyright (c) 2005 - 2013, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 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 Software License
Agreement which accompanies this distribution.
// Write LFN directory entry
//
SetMem (LfnBuffer, sizeof (CHAR16) * LFN_CHAR_TOTAL * EntryCount, 0xff);
Why do you set the whole buffer to 0xFF? What's the value here?
- StrCpy (LfnBuffer, DirEnt->FileString);
+ Status = StrCpyS (
+ LfnBuffer,
+ sizeof (LfnBuffer) / sizeof (LfnBuffer[0]),
+ DirEnt->FileString
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
LfnBufferPointer = LfnBuffer;
LfnEntry.Attributes = FAT_ATTRIBUTE_LFN;
LfnEntry.Type = 0;
// Fail to get the long file name from long file name entry,
// get the file name from short name
//
- FatGetFileNameViaCaseFlag (DirEnt, LfnBuffer);
+ FatGetFileNameViaCaseFlag (
+ DirEnt,
+ LfnBuffer,
+ sizeof (LfnBuffer) / sizeof (LfnBuffer[0])
+ );
}
DirEnt->FileString = AllocateCopyPool (StrSize (LfnBuffer), LfnBuffer); diff --
git a/EnhancedFatDxe/Fat.h b/EnhancedFatDxe/Fat.h index
7a3cd06..27d7937 100644
--- a/EnhancedFatDxe/Fat.h
+++ b/EnhancedFatDxe/Fat.h
@@ -1237,8 +1237,9 @@ FatSetCaseFlag (
VOID
FatGetFileNameViaCaseFlag (
- IN FAT_DIRENT *DirEnt,
- OUT CHAR16 *FileString
+ IN FAT_DIRENT *DirEnt,
+ IN OUT CHAR16 *FileString,
+ IN UINTN FileStringMax
);
UINT8
diff --git a/EnhancedFatDxe/FileName.c b/EnhancedFatDxe/FileName.c
index 1ba2706..1f8aad2 100644
--- a/EnhancedFatDxe/FileName.c
+++ b/EnhancedFatDxe/FileName.c
@@ -1,6 +1,6 @@
/*++
-Copyright (c) 2005 - 2007, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 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 Software License
Agreement which accompanies this distribution.
CHAR16 Buffer[FAT_MAIN_NAME_LEN + 1 + FAT_EXTEND_NAME_LEN + 1];
UINT8 OutCaseFlag;
- ASSERT (StrSize (Str) <= sizeof (Buffer));
//
// Assume the case of input string is mixed
//
// Lower case a copy of the string, if it matches the
// original then the string is lower case
//
- StrCpy (Buffer, Str);
+ StrCpyS (Buffer, sizeof (Buffer) / sizeof (Buffer[0]), Str);
FatStrLwr (Buffer);
if (StrCmp (Str, Buffer) == 0) {
OutCaseFlag = InCaseFlag;
// Upper case a copy of the string, if it matches the
// original then the string is upper case
//
- StrCpy (Buffer, Str);
+ StrCpyS (Buffer, sizeof (Buffer) / sizeof (Buffer[0]), Str);
FatStrUpr (Buffer);
if (StrCmp (Str, Buffer) == 0) {
OutCaseFlag = 0;
VOID
FatGetFileNameViaCaseFlag (
- IN FAT_DIRENT *DirEnt,
- OUT CHAR16 *FileString
+ IN FAT_DIRENT *DirEnt,
+ IN OUT CHAR16 *FileString,
+ IN UINTN FileStringMax
)
/*++
FatNameToStr (File8Dot3Name + FAT_MAIN_NAME_LEN,
FAT_EXTEND_NAME_LEN, CaseFlag & FAT_CASE_EXT_LOWER,
&TempExt[1]);
if (TempExt[1] != 0) {
TempExt[0] = L'.';
- StrCat (FileString, TempExt);
+ StrCatS (FileString, FileStringMax, TempExt);
}
}
diff --git a/EnhancedFatDxe/Hash.c b/EnhancedFatDxe/Hash.c index
d186e35..d933f90 100644
--- a/EnhancedFatDxe/Hash.c
+++ b/EnhancedFatDxe/Hash.c
@@ -1,6 +1,6 @@
/*++
-Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 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 Software License
Agreement which accompanies this distribution.
{
UINT32 HashValue;
CHAR16 UpCasedLongFileName[EFI_PATH_STRING_LENGTH];
- StrnCpy (UpCasedLongFileName, LongNameString,
EFI_PATH_STRING_LENGTH - 1);
- UpCasedLongFileName[EFI_PATH_STRING_LENGTH - 1] = L'\0';
+ StrCpyS (
+ UpCasedLongFileName,
+ sizeof (UpCasedLongFileName) / sizeof (UpCasedLongFileName[0]),
+ LongNameString
+ );
FatStrUpr (UpCasedLongFileName);
gBS->CalculateCrc32 (UpCasedLongFileName, StrSize
(UpCasedLongFileName), &HashValue);
return (HashValue & HASH_TABLE_MASK);
--
1.9.5.msysgit.0
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Loading...