From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by mx.groups.io with SMTP id smtpd.web08.2550.1626811906661875764 for ; Tue, 20 Jul 2021 13:11:46 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=iP/gKbrF; spf=pass (domain: nuviainc.com, ip: 209.85.216.50, mailfrom: rebecca@nuviainc.com) Received: by mail-pj1-f50.google.com with SMTP id g22-20020a17090a5796b02901763aca3df6so274361pji.5 for ; Tue, 20 Jul 2021 13:11:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-transfer-encoding:content-language; bh=3QxO6D+OaZDoOJS45Pr0wsW/MwI8sJMXy8xJ3NgxCrM=; b=iP/gKbrFaVl4SpkSTrwYdrZ9o95z6rEH1x2CKZanZpzn52CJN41HTdXBQndNYCYxhP LQz1+QJuGxMcQqnrkkiJnBs9LHFcDRE0hUQscRrd0Hy2u8Dl6+U2ZIJWq04GSBKHL9EG /CuHh3vtyTL//6aRlXiZBstB2VvmNPpnANDIyNwuIW3jBh2YPWu9O0/b9GWN0vy4sK8u jyyF0pYhruOpckq9rmG27mj+sH6EO6wCAnMQ+YkS+iNzRkZtB57rE4sHfMYMqbNNyoV5 2iLowBvWGNSkib/uQGipsP6ctUT8NDMXAij7CTeQdTbzwI4jgGumKJ+rc/REA5htE0R7 HRrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=3QxO6D+OaZDoOJS45Pr0wsW/MwI8sJMXy8xJ3NgxCrM=; b=tlsBWMBGDXA1fFbIwlX04DFgfny0HeCxzXjeWP997CiukO8qytmqRjJ8kdrmGexQNU 5PmW9lK4RxBBIli+SjeAS9tup97F8IJ7HfZbj+B4JlhWXOTr4GlDDxJIQy1sMTSYNwto 9VsG8eGl5iJd3kkzbW6xggVL+NaAhRaWCgeVRpnTvxwXdR8WQCc4YaKxVFYY9uWhtHVf 3g86kAu+u8FmY65r1/EE/MTo4CcKOeaD2MzrWrG0fTEnOvB+gs1PYFw26NH4mm602Vje dGEi5X5v8Hm6AYprBJu4CyeX1FIL8S/y7chLgnUiJUnxexmdfxlFwemhHM1LZgxoTuOx N3zQ== X-Gm-Message-State: AOAM5302zyTN8QDxBdUoe1kKJPUh5IqFOFMzXHqD980CsB/b2tcXxVF9 AAc+UyArJM3b2h0b26LCBi+yQA== X-Google-Smtp-Source: ABdhPJyeMXuX80uqxEt+1UDDjvYvahEs8IM7aIRFSWiiNJ2YkfDabmSMmKRIiqffNHYbBJykWS/H6g== X-Received: by 2002:a17:902:fe0a:b029:11d:81c9:3adf with SMTP id g10-20020a170902fe0ab029011d81c93adfmr24965751plj.0.1626811906101; Tue, 20 Jul 2021 13:11:46 -0700 (PDT) Return-Path: Received: from linbox.int.bluestop.org (c-174-52-16-57.hsd1.ut.comcast.net. [174.52.16.57]) by smtp.gmail.com with ESMTPSA id x40sm25296933pfu.176.2021.07.20.13.11.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 20 Jul 2021 13:11:45 -0700 (PDT) Subject: Re: [edk2-devel] [edk2-libc PATCH 1/1] Update LibC to use safe string functions To: devel@edk2.groups.io, Daryl McDaniel , Jaben Carsey References: <1686594EFA3CDA35.1617@groups.io> From: "Rebecca Cran" Message-ID: Date: Tue, 20 Jul 2021 14:11:43 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <1686594EFA3CDA35.1617@groups.io> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US This hasn't had any reviews yet. Could someone take a look at it please? Thanks. Rebecca Cran On 6/7/21 10:21 AM, Rebecca Cran via groups.io wrote: > The insecure string functions such as StrCpy were removed a while ago, > breaking the StdLib build. Migrate StdLib/LibC to the safe string > versions. > > Signed-off-by: Rebecca Cran > --- > StdLib/LibC/StdLib/Environs.c | 11 ++++++----- > StdLib/LibC/StdLib/realpath.c | 5 +++-- > StdLib/LibC/String/Concatenation.c | 7 +++++-- > StdLib/LibC/String/Copying.c | 7 +++++-- > StdLib/LibC/Uefi/Devices/Utility/Path.c | 3 ++- > StdLib/LibC/Uefi/SysCalls.c | 5 +++-- > StdLib/LibC/Wchar/Concatenation.c | 6 ++++-- > StdLib/LibC/Wchar/Copying.c | 4 ++-- > 8 files changed, 30 insertions(+), 18 deletions(-) > > diff --git a/StdLib/LibC/StdLib/Environs.c b/StdLib/LibC/StdLib/Environs.c > index a29cb9954cf9..ad56629554df 100644 > --- a/StdLib/LibC/StdLib/Environs.c > +++ b/StdLib/LibC/StdLib/Environs.c > @@ -151,7 +151,7 @@ system(const char *string) > if( string == NULL) { > return 1; > } > - (void)AsciiStrToUnicodeStr( string, gMD->UString); > + (void)AsciiStrToUnicodeStrS (string, gMD->UString, UNICODE_STRING_MAX); > OpStat = ShellExecute( &MyHandle, gMD->UString, FALSE, NULL, &CmdStat); > if(OpStat == RETURN_SUCCESS) { > EFIerrno = CmdStat; > @@ -177,10 +177,11 @@ char *getenv(const char *name) > const CHAR16 *EfiEnv; > char *retval = NULL; > > - (void)AsciiStrToUnicodeStr( name, gMD->UString); > + (void)AsciiStrToUnicodeStrS (name, gMD->UString, UNICODE_STRING_MAX); > EfiEnv = ShellGetEnvironmentVariable(gMD->UString); > if(EfiEnv != NULL) { > - retval = UnicodeStrToAsciiStr( EfiEnv, gMD->ASgetenv); > + (void)UnicodeStrToAsciiStrS (EfiEnv, gMD->ASgetenv, UNICODE_STRING_MAX); > + retval = gMD->ASgetenv; > } > > return retval; > @@ -238,8 +239,8 @@ setenv ( > // > // Convert the strings > // > - AsciiStrToUnicodeStr ( name, UName ); > - AsciiStrToUnicodeStr ( value, UValue ); > + AsciiStrToUnicodeStrS (name, UName, UNICODE_STRING_MAX); > + AsciiStrToUnicodeStrS (value, UValue, UNICODE_STRING_MAX); > > // > // Determine if the string is already present > diff --git a/StdLib/LibC/StdLib/realpath.c b/StdLib/LibC/StdLib/realpath.c > index 6d75f17a394d..a8ff1e9d5b1d 100644 > --- a/StdLib/LibC/StdLib/realpath.c > +++ b/StdLib/LibC/StdLib/realpath.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > > /** The realpath() function shall derive, from the pathname pointed to by > file_name, an absolute pathname that names the same file, whose resolution > @@ -47,8 +48,8 @@ realpath( > errno = ENOMEM; > return (NULL); > } > - AsciiStrToUnicodeStr(file_name, Temp); > + AsciiStrToUnicodeStrS (file_name, Temp, UNICODE_STRING_MAX); > PathCleanUpDirectories(Temp); > - UnicodeStrToAsciiStr(Temp, resolved_name); > + UnicodeStrToAsciiStrS (Temp, resolved_name, UNICODE_STRING_MAX); > return (resolved_name); > } > diff --git a/StdLib/LibC/String/Concatenation.c b/StdLib/LibC/String/Concatenation.c > index e76bea0bf858..f78836fbe0d6 100644 > --- a/StdLib/LibC/String/Concatenation.c > +++ b/StdLib/LibC/String/Concatenation.c > @@ -15,6 +15,7 @@ > > #include > > +#include > #include > > /** The strcat function appends a copy of the string pointed to by s2 > @@ -28,7 +29,8 @@ > char * > strcat(char * __restrict s1, const char * __restrict s2) > { > - return AsciiStrCat( s1, s2); > + AsciiStrCatS (s1, UNICODE_STRING_MAX, s2); > + return s1; > } > > /** The strncat function appends not more than n characters (a null character > @@ -43,7 +45,8 @@ strcat(char * __restrict s1, const char * __restrict s2) > char * > strncat(char * __restrict s1, const char * __restrict s2, size_t n) > { > - return AsciiStrnCat( s1, s2, n); > + AsciiStrnCatS (s1, UNICODE_STRING_MAX, s2, n); > + return s1; > } > > /** The strncatX function appends not more than n characters (a null character > diff --git a/StdLib/LibC/String/Copying.c b/StdLib/LibC/String/Copying.c > index 3234eccf0808..cc2077a5b80a 100644 > --- a/StdLib/LibC/String/Copying.c > +++ b/StdLib/LibC/String/Copying.c > @@ -16,6 +16,7 @@ > > #include > > +#include > #include > #include > > @@ -73,7 +74,8 @@ strcpy(char * __restrict s1, const char * __restrict s2) > > //while ( *s1++ = *s2++) /* Empty Body */; > //return(s1ret); > - return AsciiStrCpy( s1, s2); > + AsciiStrCpyS (s1, UNICODE_STRING_MAX, s2); > + return s1; > } > > /** The strncpy function copies not more than n characters (characters that > @@ -89,7 +91,8 @@ strcpy(char * __restrict s1, const char * __restrict s2) > **/ > char *strncpy(char * __restrict s1, const char * __restrict s2, size_t n) > { > - return AsciiStrnCpy( s1, s2, n); > + AsciiStrnCpyS (s1, UNICODE_STRING_MAX, s2, n); > + return s1; > //char *dest = s1; > > //while(n != 0) { > diff --git a/StdLib/LibC/Uefi/Devices/Utility/Path.c b/StdLib/LibC/Uefi/Devices/Utility/Path.c > index 96392e018dac..d6728d3a647e 100644 > --- a/StdLib/LibC/Uefi/Devices/Utility/Path.c > +++ b/StdLib/LibC/Uefi/Devices/Utility/Path.c > @@ -110,7 +110,8 @@ NormalizePath( const char *path) > wchar_t *NewPath; > size_t Length; > > - OldPath = AsciiStrToUnicodeStr(path, gMD->UString); > + AsciiStrToUnicodeStrS (path, gMD->UString, UNICODE_STRING_MAX); > + OldPath = gMD->UString; > Length = wcslen(OldPath) + 1; > > NewPath = calloc(Length, sizeof(wchar_t)); > diff --git a/StdLib/LibC/Uefi/SysCalls.c b/StdLib/LibC/Uefi/SysCalls.c > index faa73ed7a4ee..e83b72308fbe 100644 > --- a/StdLib/LibC/Uefi/SysCalls.c > +++ b/StdLib/LibC/Uefi/SysCalls.c > @@ -1320,7 +1320,8 @@ char > errno = ERANGE; > return (NULL); > } > - return (UnicodeStrToAsciiStr(Cwd, buf)); > + UnicodeStrToAsciiStrS (Cwd, buf, UNICODE_STRING_MAX); > + return buf; > } > > /** Change the current working directory. > @@ -1358,7 +1359,7 @@ chdir (const char *path) > errno = ENOMEM; > return -1; > } > - AsciiStrToUnicodeStr(path, UnicodePath); > + AsciiStrToUnicodeStrS (path, UnicodePath, UNICODE_STRING_MAX); > Status = gEfiShellProtocol->SetCurDir(NULL, UnicodePath); > FreePool(UnicodePath); > if (EFI_ERROR(Status)) { > diff --git a/StdLib/LibC/Wchar/Concatenation.c b/StdLib/LibC/Wchar/Concatenation.c > index cf595a461f0e..7289240951aa 100644 > --- a/StdLib/LibC/Wchar/Concatenation.c > +++ b/StdLib/LibC/Wchar/Concatenation.c > @@ -31,7 +31,8 @@ > **/ > wchar_t *wcscat(wchar_t * __restrict s1, const wchar_t * __restrict s2) > { > - return (wchar_t *)StrCat( (CHAR16 *)s1, (CONST CHAR16 *)s2); > + StrCatS ((CHAR16 *)s1, UNICODE_STRING_MAX, (CONST CHAR16 *)s2); > + return s1; > } > > /** The wcsncat function appends not more than n wide characters (a null wide > @@ -44,5 +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) > { > - return (wchar_t *)StrnCat( (CHAR16 *)s1, (CONST CHAR16 *)s2, (UINTN)n); > + StrnCatS ((CHAR16 *)s1, UNICODE_STRING_MAX, (CONST CHAR16 *)s2, (UINTN)n); > + return s1; > } > diff --git a/StdLib/LibC/Wchar/Copying.c b/StdLib/LibC/Wchar/Copying.c > index 7075437965ad..848c83419ddb 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 *)StrCpy( (CHAR16 *)s1, (CONST CHAR16 *)s2); > + return (wchar_t *)StrCpyS ((CHAR16 *)s1, UNICODE_STRING_MAX, (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 *)StrnCpy( (CHAR16 *)s1, (CONST CHAR16 *)s2, (UINTN)n); > + return (wchar_t *)StrnCpyS ((CHAR16 *)s1, UNICODE_STRING_MAX, (CONST CHAR16 *)s2, (UINTN)n); > } > > /** The wmemcpy function copies n wide characters from the object pointed to by