Re: [PATCH] MdePkg/BasePrintLib: Fix incorrect Precision position calculation

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Is the commit log correct?

Is the issue that the character past the '\0' could be read?

Mike

> -----Original Message-----
> From: Wang, Jian J
> Sent: Tuesday, December 26, 2017 6:14 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-
> devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [PATCH] MdePkg/BasePrintLib: Fix
> incorrect Precision position calculation
> 
> Mike and Liming,
> 
> Could you take a look at this patch?
> 
> Regards,
> Jian
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Jian J
> > Wang
> > Sent: Monday, December 25, 2017 10:09 AM
> > To: edk2-devel@lists.01.org
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Yao, Jiewen
> > <jiewen.yao@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Gao, Liming
> > <liming.gao@intel.com>
> > Subject: [edk2] [PATCH] MdePkg/BasePrintLib: Fix
> incorrect Precision position
> > calculation
> >
> > Due to the a potential hole in the stop condition of
> for-loop, the two
> > continuous access to ArgumentString (index, index+1)
> inside the loop
> > might cause the string ending character ('\0') to be
> read.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> >  MdePkg/Library/BasePrintLib/PrintLibInternal.c | 5
> ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git
> a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
> > b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
> > index 28d946472f..297d5a05b5 100644
> > --- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
> > +++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
> > @@ -1107,7 +1107,10 @@ BasePrintLibSPrintMarker (
> >        // Compute the number of characters in
> ArgumentString and store it in
> > Count
> >        // ArgumentString is either null-terminated, or
> it contains Precision
> > characters
> >        //
> > -      for (Count = 0; Count < Precision || ((Flags &
> PRECISION) == 0); Count++) {
> > +      for (Count = 0;
> > +            ArgumentString[Count *
> BytesPerArgumentCharacter] != '\0' &&
> > +            (Count < Precision || ((Flags & PRECISION)
> == 0));
> > +              Count++) {
> >          ArgumentCharacter = ((ArgumentString[Count *
> > BytesPerArgumentCharacter] & 0xff) |
> ((ArgumentString[Count *
> > BytesPerArgumentCharacter + 1]) << 8)) & ArgumentMask;
> >          if (ArgumentCharacter == 0) {
> >            break;
> > --
> > 2.15.1.windows.2
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel