Re: [edk2-devel] [PATCH] CryptoPkg: Fix BaseCryptLib CrtWrapper strcpy

["Witt, Sebastian via groups.io" <sebastian.witt=siemens.com@groups.io>]

Ok thanks, I'll look at the other wrappers and do a PR.

Regards,
Sebastian

On Tue, 2024-06-04 at 07:28 +0000, Li, Yi1 wrote:
> Thanks for your patch, this is a known issue:
> https://bugzilla.tianocore.org/show_bug.cgi?id=2817
>
> Could you also update other wrappers in CrtWrapper.h and add BZ link to commit message?
>
> Edk2 has switched to github pr code review process, you can raise PR in
> https://github.com/tianocore/edk2/pulls
>  directly.
>
> Regards,
> Yi
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Witt, Sebastian via groups.io
> Sent: Tuesday, June 4, 2024 12:19 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH] CryptoPkg: Fix BaseCryptLib CrtWrapper strcpy
>
>
> strcpy fails when strSource is closer than 4096 bytes after strDest.
>
> This is caused by an overlap check in AsciiStrCpyS:
>   //
>   // 5. Copying shall not take place between objects that overlap.
>   //
>   SAFE_STRING_CONSTRAINT_CHECK (InternalSafeStringNoAsciiStrOverlap (Destination, DestMax, (CHAR8 *)Source, SourceLen + 1), RETURN_ACCESS_DENIED);
>
> Since DestMax is MAX_STRING_SIZE (0x1000) and with a Source that is in this area behind Destination, AsciiStrCpyS will fail and strcpy will do nothing.
>
> When called by CRYPTO_strdup in openssl this leads to uninitialzed memory that gets accessed instead of the copied string.
>
> Signed-of-by: Sebastian Witt <sebastian.witt@siemens.com>
> ---
>  CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> index 37cdecc9bd..880ed140fd 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> @@ -271,7 +271,7 @@ strcpy (
>    const char  *strSource
>    )
>  {
> -  AsciiStrCpyS (strDest, MAX_STRING_SIZE, strSource);
> +  AsciiStrCpyS (strDest, AsciiStrnSizeS (strSource, MAX_STRING_SIZE),
> + strSource);
>    return strDest;
>  }
>
> --
> 2.39.2
>
>
>
> 
>
>

--
Sebastian Witt
Siemens AG
http://www.siemens.com/


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119444): https://edk2.groups.io/g/devel/message/119444
Mute This Topic: https://groups.io/mt/106471263/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-