1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v2 0/8] support server identity validation in HTTPS Boot (CVE-2019-14553)
  4. View patch

[edk2-devel] [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553)

Laszlo Ersek posted 8 patches 6 years, 3 months ago
[edk2-devel] [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553)
Posted by Laszlo Ersek 6 years, 3 months ago 
According to the ISO C standard, strchr() is a function. We #define it as
a macro. Unfortunately, our macro evaluates the first argument ("str")
twice. If the expression passed for "str" has side effects, the behavior
may be undefined.

In a later patch in this series, we're going to resurrect "inet_pton.c"
(originally from the StdLib package), which calls strchr() just like that:

  strchr((xdigits = xdigits_l), ch)
  strchr((xdigits = xdigits_u), ch)

To enable this kind of function call, turn strchr() into a function.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
CVE: CVE-2019-14553
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - new patch

 CryptoPkg/Library/Include/CrtLibSupport.h           | 2 +-
 CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h b/CryptoPkg/Library/Include/CrtLibSupport.h
index 5806f50f7485..b90da20ff7e7 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -146,8 +146,9 @@ int            isalnum     (int);
 int            isupper     (int);
 int            tolower     (int);
 int            strcmp      (const char *, const char *);
 int            strncasecmp (const char *, const char *, size_t);
+char           *strchr     (const char *, int);
 char           *strrchr    (const char *, int);
 unsigned long  strtoul     (const char *, char **, int);
 long           strtol      (const char *, char **, int);
 char           *strerror   (int);
@@ -187,9 +188,8 @@ void           abort       (void);
 #define strlen(str)                       (size_t)(AsciiStrnLenS(str,MAX_STRING_SIZE))
 #define strcpy(strDest,strSource)         AsciiStrCpyS(strDest,MAX_STRING_SIZE,strSource)
 #define strncpy(strDest,strSource,count)  AsciiStrnCpyS(strDest,MAX_STRING_SIZE,strSource,(UINTN)count)
 #define strcat(strDest,strSource)         AsciiStrCatS(strDest,MAX_STRING_SIZE,strSource)
-#define strchr(str,ch)                    ScanMem8((VOID *)(str),AsciiStrSize(str),(UINT8)ch)
 #define strncmp(string1,string2,count)    (int)(AsciiStrnCmp(string1,string2,(UINTN)(count)))
 #define strcasecmp(str1,str2)             (int)AsciiStriCmp(str1,str2)
 #define sprintf(buf,...)                  AsciiSPrint(buf,MAX_STRING_SIZE,__VA_ARGS__)
 #define localtime(timer)                  NULL
diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
index 71a2ef34ed2b..42235ab96ac3 100644
--- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
+++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
@@ -114,8 +114,13 @@ QuickSortWorker (
 //
 // -- String Manipulation Routines --
 //
 
+char *strchr(const char *str, int ch)
+{
+  return ScanMem8 (str, AsciiStrSize (str), (UINT8)ch);
+}
+
 /* Scan a string for the last occurrence of a character */
 char *strrchr (const char *str, int c)
 {
   char * save;
-- 
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#49465): https://edk2.groups.io/g/devel/message/49465
Mute This Topic: https://groups.io/mt/37952587/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553)
Posted by Philippe Mathieu-Daudé 6 years, 3 months ago 
On 10/26/19 7:37 AM, Laszlo Ersek wrote:
> According to the ISO C standard, strchr() is a function. We #define it as
> a macro. Unfortunately, our macro evaluates the first argument ("str")
> twice. If the expression passed for "str" has side effects, the behavior
> may be undefined.
> 
> In a later patch in this series, we're going to resurrect "inet_pton.c"
> (originally from the StdLib package), which calls strchr() just like that:
> 
>    strchr((xdigits = xdigits_l), ch)
>    strchr((xdigits = xdigits_u), ch)
> 
> To enable this kind of function call, turn strchr() into a function.
> 
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
> CVE: CVE-2019-14553
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>      v2:
>      - new patch
> 
>   CryptoPkg/Library/Include/CrtLibSupport.h           | 2 +-
>   CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 5 +++++
>   2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h b/CryptoPkg/Library/Include/CrtLibSupport.h
> index 5806f50f7485..b90da20ff7e7 100644
> --- a/CryptoPkg/Library/Include/CrtLibSupport.h
> +++ b/CryptoPkg/Library/Include/CrtLibSupport.h
> @@ -146,8 +146,9 @@ int            isalnum     (int);
>   int            isupper     (int);
>   int            tolower     (int);
>   int            strcmp      (const char *, const char *);
>   int            strncasecmp (const char *, const char *, size_t);
> +char           *strchr     (const char *, int);
>   char           *strrchr    (const char *, int);
>   unsigned long  strtoul     (const char *, char **, int);
>   long           strtol      (const char *, char **, int);
>   char           *strerror   (int);
> @@ -187,9 +188,8 @@ void           abort       (void);
>   #define strlen(str)                       (size_t)(AsciiStrnLenS(str,MAX_STRING_SIZE))
>   #define strcpy(strDest,strSource)         AsciiStrCpyS(strDest,MAX_STRING_SIZE,strSource)
>   #define strncpy(strDest,strSource,count)  AsciiStrnCpyS(strDest,MAX_STRING_SIZE,strSource,(UINTN)count)
>   #define strcat(strDest,strSource)         AsciiStrCatS(strDest,MAX_STRING_SIZE,strSource)
> -#define strchr(str,ch)                    ScanMem8((VOID *)(str),AsciiStrSize(str),(UINT8)ch)
>   #define strncmp(string1,string2,count)    (int)(AsciiStrnCmp(string1,string2,(UINTN)(count)))
>   #define strcasecmp(str1,str2)             (int)AsciiStriCmp(str1,str2)
>   #define sprintf(buf,...)                  AsciiSPrint(buf,MAX_STRING_SIZE,__VA_ARGS__)
>   #define localtime(timer)                  NULL
> diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> index 71a2ef34ed2b..42235ab96ac3 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> @@ -114,8 +114,13 @@ QuickSortWorker (
>   //
>   // -- String Manipulation Routines --
>   //
>   
> +char *strchr(const char *str, int ch)
> +{
> +  return ScanMem8 (str, AsciiStrSize (str), (UINT8)ch);
> +}
> +
>   /* Scan a string for the last occurrence of a character */
>   char *strrchr (const char *str, int c)
>   {
>     char * save;
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#49471): https://edk2.groups.io/g/devel/message/49471
Mute This Topic: https://groups.io/mt/37952587/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553)
Posted by Wang, Jian J 6 years, 3 months ago 
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Saturday, October 26, 2019 1:37 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>
> Cc: David Woodhouse <dwmw2@infradead.org>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Sivaraman Nainar
> <sivaramann@amiindia.co.in>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: [edk2-devel] [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function
> (CVE-2019-14553)
> 
> According to the ISO C standard, strchr() is a function. We #define it as
> a macro. Unfortunately, our macro evaluates the first argument ("str")
> twice. If the expression passed for "str" has side effects, the behavior
> may be undefined.
> 
> In a later patch in this series, we're going to resurrect "inet_pton.c"
> (originally from the StdLib package), which calls strchr() just like that:
> 
>   strchr((xdigits = xdigits_l), ch)
>   strchr((xdigits = xdigits_u), ch)
> 
> To enable this kind of function call, turn strchr() into a function.
> 
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Sivaraman Nainar <sivaramann@amiindia.co.in>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960
> CVE: CVE-2019-14553
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - new patch
> 
>  CryptoPkg/Library/Include/CrtLibSupport.h           | 2 +-
>  CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h
> b/CryptoPkg/Library/Include/CrtLibSupport.h
> index 5806f50f7485..b90da20ff7e7 100644
> --- a/CryptoPkg/Library/Include/CrtLibSupport.h
> +++ b/CryptoPkg/Library/Include/CrtLibSupport.h
> @@ -146,8 +146,9 @@ int            isalnum     (int);
>  int            isupper     (int);
>  int            tolower     (int);
>  int            strcmp      (const char *, const char *);
>  int            strncasecmp (const char *, const char *, size_t);
> +char           *strchr     (const char *, int);
>  char           *strrchr    (const char *, int);
>  unsigned long  strtoul     (const char *, char **, int);
>  long           strtol      (const char *, char **, int);
>  char           *strerror   (int);
> @@ -187,9 +188,8 @@ void           abort       (void);
>  #define strlen(str)                       (size_t)(AsciiStrnLenS(str,MAX_STRING_SIZE))
>  #define strcpy(strDest,strSource)
> AsciiStrCpyS(strDest,MAX_STRING_SIZE,strSource)
>  #define strncpy(strDest,strSource,count)
> AsciiStrnCpyS(strDest,MAX_STRING_SIZE,strSource,(UINTN)count)
>  #define strcat(strDest,strSource)
> AsciiStrCatS(strDest,MAX_STRING_SIZE,strSource)
> -#define strchr(str,ch)                    ScanMem8((VOID
> *)(str),AsciiStrSize(str),(UINT8)ch)
>  #define strncmp(string1,string2,count)
> (int)(AsciiStrnCmp(string1,string2,(UINTN)(count)))
>  #define strcasecmp(str1,str2)             (int)AsciiStriCmp(str1,str2)
>  #define sprintf(buf,...)
> AsciiSPrint(buf,MAX_STRING_SIZE,__VA_ARGS__)
>  #define localtime(timer)                  NULL
> diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> index 71a2ef34ed2b..42235ab96ac3 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c
> @@ -114,8 +114,13 @@ QuickSortWorker (
>  //
>  // -- String Manipulation Routines --
>  //
> 
> +char *strchr(const char *str, int ch)
> +{
> +  return ScanMem8 (str, AsciiStrSize (str), (UINT8)ch);
> +}
> +
>  /* Scan a string for the last occurrence of a character */
>  char *strrchr (const char *str, int c)
>  {
>    char * save;
> --
> 2.19.1.3.g30247aa5d201
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#49536): https://edk2.groups.io/g/devel/message/49536
Mute This Topic: https://groups.io/mt/37952587/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.