REF:https://bugzilla.tianocore.org/show_bug.cgi?id=828
The commit refines the boundary checks for file/path name string to
prevent possible buffer overrun.
Cc: Paulo Alcantara <pa...@paulo.ac>
Cc: Ruiyu Ni <ruiyu...@intel.com>
Cc: Jiewen Yao <jiewen....@intel.com>
Cc: Star Zeng <star.z...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a...@intel.com>
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 29 +++++++--
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 64
+++++++++++++++++---
MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 29 ++++++++-
3 files changed, 107 insertions(+), 15 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 6f07bf2066..80db734f86 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -248,7 +248,7 @@ UdfOpen (
FileName = TempFileName + 1;
}
- StrCpyS (NewPrivFileData->FileName, UDF_PATH_LENGTH, FileName);
+ StrCpyS (NewPrivFileData->FileName, UDF_FILENAME_LENGTH, FileName);
Status = GetFileSize (
PrivFsData->BlockIo,
@@ -444,7 +444,7 @@ UdfRead (
FreePool ((VOID *)NewFileEntryData);
NewFileEntryData = FoundFile.FileEntry;
- Status = GetFileNameFromFid (NewFileIdentifierDesc, FileName);
+ Status = GetFileNameFromFid (NewFileIdentifierDesc, ARRAY_SIZE
(FileName), FileName);
if (EFI_ERROR (Status)) {
FreePool ((VOID *)FoundFile.FileIdentifierDesc);
goto Error_Get_FileName;
@@ -456,7 +456,7 @@ UdfRead (
FoundFile.FileIdentifierDesc = NewFileIdentifierDesc;
FoundFile.FileEntry = NewFileEntryData;
- Status = GetFileNameFromFid (FoundFile.FileIdentifierDesc, FileName);
+ Status = GetFileNameFromFid (FoundFile.FileIdentifierDesc, ARRAY_SIZE
(FileName), FileName);
if (EFI_ERROR (Status)) {
goto Error_Get_FileName;
}
@@ -718,6 +718,12 @@ UdfSetPosition (
/**
Get information about a file.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The File Set Descriptor is external input, so this routine will do basic
+ validation for File Set Descriptor and report status.
+
@param This Protocol instance pointer.
@param InformationType Type of information to return in Buffer.
@param BufferSize On input size of buffer, on output amount of data in
@@ -794,6 +800,10 @@ UdfGetInfo (
*String = *(UINT8 *)(OstaCompressed + Index) << 8;
Index++;
} else {
+ if (Index > ARRAY_SIZE (VolumeLabel)) {
+ return EFI_VOLUME_CORRUPTED;
+ }
+
*String = 0;
}
@@ -813,7 +823,11 @@ UdfGetInfo (
String++;
}
- *String = L'\0';
+ Index = ((UINTN)String - (UINTN)VolumeLabel) / sizeof (CHAR16);
+ if (Index > ARRAY_SIZE (VolumeLabel) - 1) {
+ Index = ARRAY_SIZE (VolumeLabel) - 1;
+ }
+ VolumeLabel[Index] = L'\0';
FileSystemInfoLength = StrSize (VolumeLabel) +
sizeof (EFI_FILE_SYSTEM_INFO);
@@ -823,8 +837,11 @@ UdfGetInfo (
}
FileSystemInfo = (EFI_FILE_SYSTEM_INFO *)Buffer;
- StrCpyS (FileSystemInfo->VolumeLabel, ARRAY_SIZE (VolumeLabel),
- VolumeLabel);
+ StrCpyS (
+ FileSystemInfo->VolumeLabel,
+ (*BufferSize - OFFSET_OF (EFI_FILE_SYSTEM_INFO, VolumeLabel)) / sizeof
(CHAR16),
+ VolumeLabel
+ );
Status = GetVolumeSize (
PrivFsData->BlockIo,
PrivFsData->DiskIo,
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index ecc172303e..5fb88c2ff3 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -1412,7 +1412,7 @@ InternalFindFile (
break;
}
} else {
- Status = GetFileNameFromFid (FileIdentifierDesc, FoundFileName);
+ Status = GetFileNameFromFid (FileIdentifierDesc, ARRAY_SIZE
(FoundFileName), FoundFileName);
if (EFI_ERROR (Status)) {
break;
}
@@ -1705,6 +1705,11 @@ FindFile (
while (*FilePath != L'\0') {
FileNamePointer = FileName;
while (*FilePath != L'\0' && *FilePath != L'\\') {
+ if ((((UINTN)FileNamePointer - (UINTN)FileName) / sizeof (CHAR16)) >=
+ (ARRAY_SIZE (FileName) - 1)) {
+ return EFI_NOT_FOUND;
+ }
+
*FileNamePointer++ = *FilePath++;
}
@@ -1882,22 +1887,38 @@ ReadDirectoryEntry (
Get a filename (encoded in OSTA-compressed format) from a File Identifier
Descriptor on an UDF volume.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The File Identifier Descriptor is external input, so this routine will do
+ basic validation for File Identifier Descriptor and report status.
+
@param[in] FileIdentifierDesc File Identifier Descriptor pointer.
+ @param[in] CharMax The maximum number of FileName Unicode char,
+ including terminating null char.
@param[out] FileName Decoded filename.
@retval EFI_SUCCESS Filename decoded and read.
@retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
+ @retval EFI_BUFFER_TOO_SMALL The string buffer FileName cannot hold the
+ decoded filename.
**/
EFI_STATUS
GetFileNameFromFid (
IN UDF_FILE_IDENTIFIER_DESCRIPTOR *FileIdentifierDesc,
+ IN UINTN CharMax,
OUT CHAR16 *FileName
)
{
- UINT8 *OstaCompressed;
- UINT8 CompressionId;
- UINT8 Length;
- UINTN Index;
+ UINT8 *OstaCompressed;
+ UINT8 CompressionId;
+ UINT8 Length;
+ UINTN Index;
+ CHAR16 *FileNameBak;
+
+ if (CharMax == 0) {
+ return EFI_BUFFER_TOO_SMALL;
+ }
OstaCompressed =
(UINT8 *)(
@@ -1910,10 +1931,22 @@ GetFileNameFromFid (
return EFI_VOLUME_CORRUPTED;
}
+ FileNameBak = FileName;
+
//
// Decode filename.
//
Length = FileIdentifierDesc->LengthOfFileIdentifier;
+ if (CompressionId == 16) {
+ if (((UINTN)Length >> 1) > CharMax) {
+ return EFI_BUFFER_TOO_SMALL;
+ }
+ } else {
+ if ((Length != 0) && ((UINTN)Length - 1 > CharMax)) {
+ return EFI_BUFFER_TOO_SMALL;
+ }
+ }
+
for (Index = 1; Index < Length; Index++) {
if (CompressionId == 16) {
*FileName = OstaCompressed[Index++] << 8;
@@ -1928,7 +1961,11 @@ GetFileNameFromFid (
FileName++;
}
- *FileName = L'\0';
+ Index = ((UINTN)FileName - (UINTN)FileNameBak) / sizeof (CHAR16);
+ if (Index > CharMax - 1) {
+ Index = CharMax - 1;
+ }
+ FileNameBak[Index] = L'\0';
return EFI_SUCCESS;
}
@@ -1936,6 +1973,12 @@ GetFileNameFromFid (
/**
Resolve a symlink file on an UDF volume.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The Path Component is external input, so this routine will do basic
+ validation for Path Component and report status.
+
@param[in] BlockIo BlockIo interface.
@param[in] DiskIo DiskIo interface.
@param[in] Volume UDF volume information structure.
@@ -2054,6 +2097,9 @@ ResolveSymlink (
Index) << 8;
Index++;
} else {
+ if (Index > ARRAY_SIZE (FileName)) {
+ return EFI_UNSUPPORTED;
+ }
*Char = 0;
}
@@ -2064,7 +2110,11 @@ ResolveSymlink (
Char++;
}
- *Char = L'\0';
+ Index = ((UINTN)Char - (UINTN)FileName) / sizeof (CHAR16);
+ if (Index > ARRAY_SIZE (FileName) - 1) {
+ Index = ARRAY_SIZE (FileName) - 1;
+ }
+ FileName[Index] = L'\0';
break;
}
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
index d441539d16..85bf5e7733 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
@@ -559,9 +559,16 @@ UdfSetPosition (
/**
Get information about a file.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The File Set Descriptor is external input, so this routine will do basic
+ validation for File Set Descriptor and report status.
+
@param This Protocol instance pointer.
@param InformationType Type of information to return in Buffer.
- @param BufferSize On input size of buffer, on output amount of data in
buffer.
+ @param BufferSize On input size of buffer, on output amount of data in
+ buffer.
@param Buffer The buffer to return data.
@retval EFI_SUCCESS Data was returned.
@@ -571,7 +578,8 @@ UdfSetPosition (
@retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
@retval EFI_WRITE_PROTECTED The device is write protected.
@retval EFI_ACCESS_DENIED The file was open for read only.
- @retval EFI_BUFFER_TOO_SMALL Buffer was too small; required size returned in
BufferSize.
+ @retval EFI_BUFFER_TOO_SMALL Buffer was too small; required size returned in
+ BufferSize.
**/
EFI_STATUS
@@ -769,21 +777,38 @@ ReadDirectoryEntry (
Get a filename (encoded in OSTA-compressed format) from a File Identifier
Descriptor on an UDF volume.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The File Identifier Descriptor is external input, so this routine will do
+ basic validation for File Identifier Descriptor and report status.
+
@param[in] FileIdentifierDesc File Identifier Descriptor pointer.
+ @param[in] CharMax The maximum number of FileName Unicode char,
+ including terminating null char.
@param[out] FileName Decoded filename.
@retval EFI_SUCCESS Filename decoded and read.
@retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
+ @retval EFI_BUFFER_TOO_SMALL The string buffer FileName cannot hold the
+ decoded filename.
**/
EFI_STATUS
GetFileNameFromFid (
IN UDF_FILE_IDENTIFIER_DESCRIPTOR *FileIdentifierDesc,
+ IN UINTN CharMax,
OUT CHAR16 *FileName
);
/**
Resolve a symlink file on an UDF volume.
+ @attention This is boundary function that may receive untrusted input.
+ @attention The input is from FileSystem.
+
+ The Path Component is external input, so this routine will do basic
+ validation for Path Component and report status.
+
@param[in] BlockIo BlockIo interface.
@param[in] DiskIo DiskIo interface.
@param[in] Volume UDF volume information structure.
--
2.12.0.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
