[PATCH v1 0/1] Fix integer overflow in CryptoPkg

[PATCH v1 0/1] Fix integer overflow in CryptoPkg

[Yuan Yu]

Some value uses UINT16 which is not wide enough to hold the values that
it is supposed to hold. This series fix it by using UINT32.

The changes can be seen at:
https://github.com/yyu/edk2/tree/overflow_fix_v1

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>

Yuan Yu (1):
  CryptoPkg: Fix integer overflow

 CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.37.3.998.g577e59143f-goog

[PATCH v1 1/1] CryptoPkg: Fix integer overflow

[Yuan Yu]

SECSPERDAY is 86400 which exceeds the limit of a UINT16 which is 65536.
Therefore DayRemainder cannot use UINT16. This patch makes it UINT32.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>

Signed-off-by: Yuan Yu <yuanyu@google.com>
---
 CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c b/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
index 7d28446d4b5c..bf8a5325817f 100644
--- a/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
+++ b/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
@@ -118,7 +118,7 @@ gmtime (
 {
   struct tm  *GmTime;
   UINT16     DayNo;
-  UINT16     DayRemainder;
+  UINT32     DayRemainder;
   time_t     Year;
   time_t     YearNo;
   UINT16     TotalDays;
@@ -136,7 +136,7 @@ gmtime (
   ZeroMem ((VOID *)GmTime, (UINTN)sizeof (struct tm));
 
   DayNo        = (UINT16)(*timer / SECSPERDAY);
-  DayRemainder = (UINT16)(*timer % SECSPERDAY);
+  DayRemainder = (UINT32)(*timer % SECSPERDAY);
 
   GmTime->tm_sec  = (int)(DayRemainder % SECSPERMIN);
   GmTime->tm_min  = (int)((DayRemainder % SECSPERHOUR) / SECSPERMIN);
-- 
2.37.3.998.g577e59143f-goog

Re: [PATCH v1 1/1] CryptoPkg: Fix integer overflow

[Ard Biesheuvel]

(cc Jiewen and Jian)

On Wed, 28 Sept 2022 at 09:49, Yuan Yu <yuanyu@google.com> wrote:
>
> SECSPERDAY is 86400 which exceeds the limit of a UINT16 which is 65536.
> Therefore DayRemainder cannot use UINT16. This patch makes it UINT32.
>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
>
> Signed-off-by: Yuan Yu <yuanyu@google.com>

Hello Yuan,

Thanks for the patch.

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

Since this is a CryptoPkg change, the CryptoPkg maintainers are
ultimately the ones that need to accept it, so I have added them to
cc.



> ---
>  CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c b/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
> index 7d28446d4b5c..bf8a5325817f 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
> +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
> @@ -118,7 +118,7 @@ gmtime (
>  {
>    struct tm  *GmTime;
>    UINT16     DayNo;
> -  UINT16     DayRemainder;
> +  UINT32     DayRemainder;
>    time_t     Year;
>    time_t     YearNo;
>    UINT16     TotalDays;
> @@ -136,7 +136,7 @@ gmtime (
>    ZeroMem ((VOID *)GmTime, (UINTN)sizeof (struct tm));
>
>    DayNo        = (UINT16)(*timer / SECSPERDAY);
> -  DayRemainder = (UINT16)(*timer % SECSPERDAY);
> +  DayRemainder = (UINT32)(*timer % SECSPERDAY);
>
>    GmTime->tm_sec  = (int)(DayRemainder % SECSPERMIN);
>    GmTime->tm_min  = (int)((DayRemainder % SECSPERHOUR) / SECSPERMIN);
> --
> 2.37.3.998.g577e59143f-goog
>

Re: [PATCH v1 1/1] CryptoPkg: Fix integer overflow

[Yao, Jiewen]

Merged https://github.com/tianocore/edk2/pull/3456

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Wednesday, September 28, 2022 5:20 PM
> To: Yuan Yu <yuanyu@google.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>
> Cc: devel@edk2.groups.io; Justen, Jordan L <jordan.l.justen@intel.com>;
> Laszlo Ersek <lersek@redhat.com>; Anthony Perard
> <anthony.perard@citrix.com>; Julien Grall <julien@xen.org>
> Subject: Re: [PATCH v1 1/1] CryptoPkg: Fix integer overflow
> 
> (cc Jiewen and Jian)
> 
> On Wed, 28 Sept 2022 at 09:49, Yuan Yu <yuanyu@google.com> wrote:
> >
> > SECSPERDAY is 86400 which exceeds the limit of a UINT16 which is 65536.
> > Therefore DayRemainder cannot use UINT16. This patch makes it UINT32.
> >
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Anthony Perard <anthony.perard@citrix.com>
> > Cc: Julien Grall <julien@xen.org>
> >
> > Signed-off-by: Yuan Yu <yuanyu@google.com>
> 
> Hello Yuan,
> 
> Thanks for the patch.
> 
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> 
> Since this is a CryptoPkg change, the CryptoPkg maintainers are
> ultimately the ones that need to accept it, so I have added them to
> cc.
> 
> 
> 
> > ---
> >  CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
> b/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
> > index 7d28446d4b5c..bf8a5325817f 100644
> > --- a/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
> > +++ b/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
> > @@ -118,7 +118,7 @@ gmtime (
> >  {
> >    struct tm  *GmTime;
> >    UINT16     DayNo;
> > -  UINT16     DayRemainder;
> > +  UINT32     DayRemainder;
> >    time_t     Year;
> >    time_t     YearNo;
> >    UINT16     TotalDays;
> > @@ -136,7 +136,7 @@ gmtime (
> >    ZeroMem ((VOID *)GmTime, (UINTN)sizeof (struct tm));
> >
> >    DayNo        = (UINT16)(*timer / SECSPERDAY);
> > -  DayRemainder = (UINT16)(*timer % SECSPERDAY);
> > +  DayRemainder = (UINT32)(*timer % SECSPERDAY);
> >
> >    GmTime->tm_sec  = (int)(DayRemainder % SECSPERMIN);
> >    GmTime->tm_min  = (int)((DayRemainder % SECSPERHOUR) /
> SECSPERMIN);
> > --
> > 2.37.3.998.g577e59143f-goog
> >