From: Jayaprakash Nevara <[email protected]> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3361
Update use of safe string functions that incorrectly assume that the maximum size of a string is UNICODE_STRING_MAX. This assumption is breaking some standard C applications. Cc: Rebecca Cran <[email protected]> Cc: Jayaprakash Nevara <[email protected]> Signed-off-by: Michael D Kinney <[email protected]> --- StdLib/LibC/StdLib/Environs.c | 2 +- StdLib/LibC/StdLib/realpath.c | 19 ++++++++++++++++--- StdLib/LibC/String/Concatenation.c | 4 ++-- StdLib/LibC/String/Copying.c | 5 ++--- StdLib/LibC/Uefi/Devices/Utility/Path.c | 17 +++++++++++------ StdLib/LibC/Uefi/SysCalls.c | 8 ++++++-- StdLib/LibC/Wchar/Concatenation.c | 4 ++-- StdLib/LibC/Wchar/Copying.c | 4 ++-- StdLib/PosixLib/GetPass/GetPass.c | 11 +++++++---- 9 files changed, 49 insertions(+), 25 deletions(-) diff --git a/StdLib/LibC/StdLib/Environs.c b/StdLib/LibC/StdLib/Environs.c index ad56629..e8cfd6d 100644 --- a/StdLib/LibC/StdLib/Environs.c +++ b/StdLib/LibC/StdLib/Environs.c @@ -180,7 +180,7 @@ char *getenv(const char *name) (void)AsciiStrToUnicodeStrS (name, gMD->UString, UNICODE_STRING_MAX); EfiEnv = ShellGetEnvironmentVariable(gMD->UString); if(EfiEnv != NULL) { - (void)UnicodeStrToAsciiStrS (EfiEnv, gMD->ASgetenv, UNICODE_STRING_MAX); + (void)UnicodeStrToAsciiStrS (EfiEnv, gMD->ASgetenv, ASCII_STRING_MAX); retval = gMD->ASgetenv; } diff --git a/StdLib/LibC/StdLib/realpath.c b/StdLib/LibC/StdLib/realpath.c index a8ff1e9..3d4118d 100644 --- a/StdLib/LibC/StdLib/realpath.c +++ b/StdLib/LibC/StdLib/realpath.c @@ -38,7 +38,9 @@ realpath( char *resolved_name ) { - CHAR16 *Temp; + RETURN_STATUS Status; + CHAR16 *Temp; + if (file_name == NULL || resolved_name == NULL) { errno = EINVAL; return (NULL); @@ -48,8 +50,19 @@ realpath( errno = ENOMEM; return (NULL); } - AsciiStrToUnicodeStrS (file_name, Temp, UNICODE_STRING_MAX); + Status = AsciiStrToUnicodeStrS (file_name, Temp, AsciiStrLen (file_name) + 1); + if (RETURN_ERROR (Status)) { + errno = EINVAL; + return NULL; + } + PathCleanUpDirectories(Temp); - UnicodeStrToAsciiStrS (Temp, resolved_name, UNICODE_STRING_MAX); + + Status = UnicodeStrToAsciiStrS (Temp, resolved_name, AsciiStrLen (file_name) + 1); + if (RETURN_ERROR (Status)) { + errno = EINVAL; + return NULL; + } + return (resolved_name); } diff --git a/StdLib/LibC/String/Concatenation.c b/StdLib/LibC/String/Concatenation.c index f78836f..ed13588 100644 --- a/StdLib/LibC/String/Concatenation.c +++ b/StdLib/LibC/String/Concatenation.c @@ -29,7 +29,7 @@ char * strcat(char * __restrict s1, const char * __restrict s2) { - AsciiStrCatS (s1, UNICODE_STRING_MAX, s2); + AsciiStrCatS (s1, AsciiStrLen (s1) + AsciiStrLen(s2) + 1, s2); return s1; } @@ -45,7 +45,7 @@ strcat(char * __restrict s1, const char * __restrict s2) char * strncat(char * __restrict s1, const char * __restrict s2, size_t n) { - AsciiStrnCatS (s1, UNICODE_STRING_MAX, s2, n); + AsciiStrnCatS (s1, AsciiStrLen (s1) + 1 + (UINTN)n, s2, n); return s1; } diff --git a/StdLib/LibC/String/Copying.c b/StdLib/LibC/String/Copying.c index cc2077a..c296714 100644 --- a/StdLib/LibC/String/Copying.c +++ b/StdLib/LibC/String/Copying.c @@ -16,7 +16,6 @@ #include <LibConfig.h> -#include <limits.h> #include <stdlib.h> #include <string.h> @@ -74,7 +73,7 @@ strcpy(char * __restrict s1, const char * __restrict s2) //while ( *s1++ = *s2++) /* Empty Body */; //return(s1ret); - AsciiStrCpyS (s1, UNICODE_STRING_MAX, s2); + AsciiStrCpyS (s1, AsciiStrLen (s2) + 1, s2); return s1; } @@ -91,7 +90,7 @@ strcpy(char * __restrict s1, const char * __restrict s2) **/ char *strncpy(char * __restrict s1, const char * __restrict s2, size_t n) { - AsciiStrnCpyS (s1, UNICODE_STRING_MAX, s2, n); + AsciiStrnCpyS (s1, n, s2, n); return s1; //char *dest = s1; diff --git a/StdLib/LibC/Uefi/Devices/Utility/Path.c b/StdLib/LibC/Uefi/Devices/Utility/Path.c index fe19196..be315bf 100644 --- a/StdLib/LibC/Uefi/Devices/Utility/Path.c +++ b/StdLib/LibC/Uefi/Devices/Utility/Path.c @@ -105,12 +105,17 @@ ClassifyPath( wchar_t * NormalizePath( const char *path) { - wchar_t *temp; - wchar_t *OldPath; - wchar_t *NewPath; - size_t Length; - - AsciiStrToUnicodeStrS (path, gMD->UString, UNICODE_STRING_MAX); + RETURN_STATUS Status; + wchar_t *temp; + wchar_t *OldPath; + wchar_t *NewPath; + size_t Length; + + Status = AsciiStrToUnicodeStrS (path, gMD->UString, UNICODE_STRING_MAX); + if (RETURN_ERROR (Status)) { + errno = EINVAL; + EFIerrno = Status; + } OldPath = gMD->UString; Length = wcslen(OldPath) + 1; diff --git a/StdLib/LibC/Uefi/SysCalls.c b/StdLib/LibC/Uefi/SysCalls.c index e83b723..0c8dcc1 100644 --- a/StdLib/LibC/Uefi/SysCalls.c +++ b/StdLib/LibC/Uefi/SysCalls.c @@ -1304,7 +1304,8 @@ write (int fd, const void *buf, size_t nbyte) char *getcwd (char *buf, size_t size) { - CONST CHAR16 *Cwd; + RETURN_STATUS Status; + CONST CHAR16 *Cwd; if (size == 0 || buf == NULL) { errno = EINVAL; @@ -1320,7 +1321,10 @@ char errno = ERANGE; return (NULL); } - UnicodeStrToAsciiStrS (Cwd, buf, UNICODE_STRING_MAX); + Status = UnicodeStrToAsciiStrS (Cwd, buf, size); + if (RETURN_ERROR (Status)) { + return NULL; + } return buf; } diff --git a/StdLib/LibC/Wchar/Concatenation.c b/StdLib/LibC/Wchar/Concatenation.c index 7289240..288e1d6 100644 --- a/StdLib/LibC/Wchar/Concatenation.c +++ b/StdLib/LibC/Wchar/Concatenation.c @@ -31,7 +31,7 @@ **/ wchar_t *wcscat(wchar_t * __restrict s1, const wchar_t * __restrict s2) { - StrCatS ((CHAR16 *)s1, UNICODE_STRING_MAX, (CONST CHAR16 *)s2); + StrCatS ((CHAR16 *)s1, StrLen (s1) + StrLen (s2) + 1, (CONST CHAR16 *)s2); return s1; } @@ -45,6 +45,6 @@ wchar_t *wcscat(wchar_t * __restrict s1, const wchar_t * __restrict s2) **/ wchar_t *wcsncat(wchar_t * __restrict s1, const wchar_t * __restrict s2, size_t n) { - StrnCatS ((CHAR16 *)s1, UNICODE_STRING_MAX, (CONST CHAR16 *)s2, (UINTN)n); + StrnCatS ((CHAR16 *)s1, StrLen (s1) + 1 + (UINTN)n, (CONST CHAR16 *)s2, (UINTN)n); return s1; } diff --git a/StdLib/LibC/Wchar/Copying.c b/StdLib/LibC/Wchar/Copying.c index 848c834..45ceda7 100644 --- a/StdLib/LibC/Wchar/Copying.c +++ b/StdLib/LibC/Wchar/Copying.c @@ -29,7 +29,7 @@ **/ wchar_t *wcscpy(wchar_t * __restrict s1, const wchar_t * __restrict s2) { - return (wchar_t *)StrCpyS ((CHAR16 *)s1, UNICODE_STRING_MAX, (CONST CHAR16 *)s2); + return (wchar_t *)StrCpyS ((CHAR16 *)s1, StrLen (s2) + 1, (CONST CHAR16 *)s2); } /** The wcsncpy function copies not more than n wide characters (those that @@ -44,7 +44,7 @@ wchar_t *wcscpy(wchar_t * __restrict s1, const wchar_t * __restrict s2) **/ wchar_t *wcsncpy(wchar_t * __restrict s1, const wchar_t * __restrict s2, size_t n) { - return (wchar_t *)StrnCpyS ((CHAR16 *)s1, UNICODE_STRING_MAX, (CONST CHAR16 *)s2, (UINTN)n); + return (wchar_t *)StrnCpyS ((CHAR16 *)s1, (UINTN)n, (CONST CHAR16 *)s2, (UINTN)n); } /** The wmemcpy function copies n wide characters from the object pointed to by diff --git a/StdLib/PosixLib/GetPass/GetPass.c b/StdLib/PosixLib/GetPass/GetPass.c index 8657827..f14c59e 100644 --- a/StdLib/PosixLib/GetPass/GetPass.c +++ b/StdLib/PosixLib/GetPass/GetPass.c @@ -15,14 +15,14 @@ #include <Library/MemoryAllocationLib.h> #include <Library/UefiLib.h> #include <Library/PcdLib.h> -#include <limits.h> static CHAR8 *ReturnStringAscii = NULL; char *getpass(const char *Prompt) { - BOOLEAN Ascii; - CHAR16 *ReturnString; + RETURN_STATUS Status; + BOOLEAN Ascii; + CHAR16 *ReturnString; Ascii = FALSE; @@ -38,7 +38,10 @@ char *getpass(const char *Prompt) return (NULL); } - UnicodeStrToAsciiStrS(ReturnString, ReturnStringAscii, UNICODE_STRING_MAX); + Status = UnicodeStrToAsciiStrS(ReturnString, ReturnStringAscii, StrLen (ReturnString) + 1); + if (RETURN_ERROR (Status)) { + ReturnStringAscii = NULL; + } FreePool(ReturnString); -- 2.32.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80028): https://edk2.groups.io/g/devel/message/80028 Mute This Topic: https://groups.io/mt/85287406/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
