From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.groups.io with SMTP id smtpd.web09.3999.1572090457457461824 for ; Sat, 26 Oct 2019 04:47:37 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: philmd@redhat.com) Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B1F324E832 for ; Sat, 26 Oct 2019 11:47:36 +0000 (UTC) Received: by mail-wr1-f69.google.com with SMTP id j14so2785809wrm.6 for ; Sat, 26 Oct 2019 04:47:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=TpKJUJnO45KzqWDAVVJr+344Q+Ab8a9demG7wKTjKKs=; b=rZnUfmgPUtnPfO11qw0OtG8OwPZlpw8mQB4LDIfwXBYR27APhde56ZzRLBktvmDjUd n14SuDjDswGWO2H8f36TJjMLFwgUkh9+dz6l3xUQbzJ2sLVRiOPfGBp8OjQXeB1l0zhx UrJUDDA10QGK4p0UqDunthnQxCL5CwkUHmlwtHGSKms/itAn6SXLrfV/NL/GFQu2uf2R /67bnRZQjk6C7AOcLOC2uzvHOK5GXgfthxLpOD84Z6SMYFuJo4h6CoGcHja7DCn7eLNO tL1BTcCXK1vmHViq8IuD6iBV0gjgBKJc5h50sZLZ8RWkd5KqCsEut8g103ca986AltBs 5o+A== X-Gm-Message-State: APjAAAVpUcQGb58upNpPcqwV4+8FY5sgJfSY1T6mrX85/aLL/QRJNmo0 dAYf8ggTQ4BMFmUqUu5qnv+V/mQ4O4HewUhVjyCS1Wqg+TprRz/Taw5Zyrj3wh7hyZO/4hgCSaI lipOG/yaSbsKOLg== X-Received: by 2002:a5d:6892:: with SMTP id h18mr6726210wru.370.1572090455455; Sat, 26 Oct 2019 04:47:35 -0700 (PDT) X-Google-Smtp-Source: APXvYqxjUWjY9tYPCpn+lFx1f5xl396xYbZ1ijKk7HAucNkc+zikrOhTDnPYVoNVNdvEf2+gA1AYcg== X-Received: by 2002:a5d:6892:: with SMTP id h18mr6726195wru.370.1572090455208; Sat, 26 Oct 2019 04:47:35 -0700 (PDT) Received: from [192.168.1.33] (62.red-88-21-202.staticip.rima-tde.net. [88.21.202.62]) by smtp.gmail.com with ESMTPSA id s21sm7081929wrb.31.2019.10.26.04.47.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 26 Oct 2019 04:47:34 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH v2 3/8] CryptoPkg/Crt: turn strchr() into a function (CVE-2019-14553) To: devel@edk2.groups.io, lersek@redhat.com Cc: David Woodhouse , Jian J Wang , Jiaxin Wu , Sivaraman Nainar , Xiaoyu Lu References: <20191026053719.10453-1-lersek@redhat.com> <20191026053719.10453-4-lersek@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Message-ID: <562cfa06-42b3-d683-0c46-2f26e6fbfdc1@redhat.com> Date: Sat, 26 Oct 2019 13:47:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 MIME-Version: 1.0 In-Reply-To: <20191026053719.10453-4-lersek@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > Cc: Jian J Wang > Cc: Jiaxin Wu > Cc: Sivaraman Nainar > Cc: Xiaoyu Lu > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=960 > CVE: CVE-2019-14553 > Signed-off-by: Laszlo Ersek > --- > > 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