The StrnLenS and AsciiStrnLenS functions, when presented with a string with no terminating NULL in the first MaxSize characters will check the character at String[MaxSize] before checking if Length < MaxSize. (They return the correct value, but have accessed beyond the stated limit in the process.)
Flip the order of the tests to prevent this behaviour.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org --- MdePkg/Library/BaseLib/SafeString.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c index 7c1b075..b0e1ce7 100644 --- a/MdePkg/Library/BaseLib/SafeString.c +++ b/MdePkg/Library/BaseLib/SafeString.c @@ -141,7 +141,7 @@ StrnLenS ( // String then StrnLenS returns MaxSize. At most the first MaxSize characters of String shall // be accessed by StrnLenS. // - for (Length = 0; (*String != 0) && (Length < MaxSize); String++, Length++) { + for (Length = 0; (Length < MaxSize) && (*String != 0); String++, Length++) { ; } return Length; @@ -551,7 +551,7 @@ AsciiStrnLenS ( // String then AsciiStrnLenS returns MaxSize. At most the first MaxSize characters of String shall // be accessed by AsciiStrnLenS. // - for (Length = 0; (*String != 0) && (Length < MaxSize); String++, Length++) { + for (Length = 0; (Length < MaxSize) && (*String != 0); String++, Length++) { ; } return Length;
Thank to catch this.
Reviewed by: Yao, Jiewen Jiewen.Yao@intel.com
-----Original Message----- From: Linaro-uefi [mailto:linaro-uefi-bounces@lists.linaro.org] On Behalf Of Leif Lindholm Sent: Friday, July 10, 2015 9:21 PM To: edk2-devel@lists.sourceforge.net Cc: Kinney, Michael D; Gao, Liming; linaro-uefi@lists.linaro.org Subject: [Linaro-uefi] [PATCH] MdePkg: ensure SafeString length functions don't access beyond MaxSize
The StrnLenS and AsciiStrnLenS functions, when presented with a string with no terminating NULL in the first MaxSize characters will check the character at String[MaxSize] before checking if Length < MaxSize. (They return the correct value, but have accessed beyond the stated limit in the process.)
Flip the order of the tests to prevent this behaviour.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org --- MdePkg/Library/BaseLib/SafeString.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c index 7c1b075..b0e1ce7 100644 --- a/MdePkg/Library/BaseLib/SafeString.c +++ b/MdePkg/Library/BaseLib/SafeString.c @@ -141,7 +141,7 @@ StrnLenS ( // String then StrnLenS returns MaxSize. At most the first MaxSize characters of String shall // be accessed by StrnLenS. // - for (Length = 0; (*String != 0) && (Length < MaxSize); String++, Length++) { + for (Length = 0; (Length < MaxSize) && (*String != 0); String++, + Length++) { ; } return Length; @@ -551,7 +551,7 @@ AsciiStrnLenS ( // String then AsciiStrnLenS returns MaxSize. At most the first MaxSize characters of String shall // be accessed by AsciiStrnLenS. // - for (Length = 0; (*String != 0) && (Length < MaxSize); String++, Length++) { + for (Length = 0; (Length < MaxSize) && (*String != 0); String++, + Length++) { ; } return Length; -- 2.1.4
_______________________________________________ Linaro-uefi mailing list Linaro-uefi@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-uefi
Reviewed-by: Liming Gao liming.gao@intel.com
Besides, if you expect me to commit this patch, please send the patch as attachment to me, I would like to help do it.
Thanks Liming -----Original Message----- From: Yao, Jiewen Sent: Monday, July 13, 2015 12:11 PM To: Leif Lindholm; edk2-devel@lists.sourceforge.net Cc: Kinney, Michael D; Gao, Liming; linaro-uefi@lists.linaro.org Subject: RE: [Linaro-uefi] [PATCH] MdePkg: ensure SafeString length functions don't access beyond MaxSize
Thank to catch this.
Reviewed by: Yao, Jiewen Jiewen.Yao@intel.com
-----Original Message----- From: Linaro-uefi [mailto:linaro-uefi-bounces@lists.linaro.org] On Behalf Of Leif Lindholm Sent: Friday, July 10, 2015 9:21 PM To: edk2-devel@lists.sourceforge.net Cc: Kinney, Michael D; Gao, Liming; linaro-uefi@lists.linaro.org Subject: [Linaro-uefi] [PATCH] MdePkg: ensure SafeString length functions don't access beyond MaxSize
The StrnLenS and AsciiStrnLenS functions, when presented with a string with no terminating NULL in the first MaxSize characters will check the character at String[MaxSize] before checking if Length < MaxSize. (They return the correct value, but have accessed beyond the stated limit in the process.)
Flip the order of the tests to prevent this behaviour.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org --- MdePkg/Library/BaseLib/SafeString.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c index 7c1b075..b0e1ce7 100644 --- a/MdePkg/Library/BaseLib/SafeString.c +++ b/MdePkg/Library/BaseLib/SafeString.c @@ -141,7 +141,7 @@ StrnLenS ( // String then StrnLenS returns MaxSize. At most the first MaxSize characters of String shall // be accessed by StrnLenS. // - for (Length = 0; (*String != 0) && (Length < MaxSize); String++, Length++) { + for (Length = 0; (Length < MaxSize) && (*String != 0); String++, + Length++) { ; } return Length; @@ -551,7 +551,7 @@ AsciiStrnLenS ( // String then AsciiStrnLenS returns MaxSize. At most the first MaxSize characters of String shall // be accessed by AsciiStrnLenS. // - for (Length = 0; (*String != 0) && (Length < MaxSize); String++, Length++) { + for (Length = 0; (Length < MaxSize) && (*String != 0); String++, + Length++) { ; } return Length; -- 2.1.4
_______________________________________________ Linaro-uefi mailing list Linaro-uefi@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-uefi
Sure - please find attached.
Regards,
Leif
On 13 July 2015 at 07:33, Gao, Liming liming.gao@intel.com wrote:
Reviewed-by: Liming Gao liming.gao@intel.com
Besides, if you expect me to commit this patch, please send the patch as attachment to me, I would like to help do it.
Thanks Liming -----Original Message----- From: Yao, Jiewen Sent: Monday, July 13, 2015 12:11 PM To: Leif Lindholm; edk2-devel@lists.sourceforge.net Cc: Kinney, Michael D; Gao, Liming; linaro-uefi@lists.linaro.org Subject: RE: [Linaro-uefi] [PATCH] MdePkg: ensure SafeString length functions don't access beyond MaxSize
Thank to catch this.
Reviewed by: Yao, Jiewen Jiewen.Yao@intel.com
-----Original Message----- From: Linaro-uefi [mailto:linaro-uefi-bounces@lists.linaro.org] On Behalf Of Leif Lindholm Sent: Friday, July 10, 2015 9:21 PM To: edk2-devel@lists.sourceforge.net Cc: Kinney, Michael D; Gao, Liming; linaro-uefi@lists.linaro.org Subject: [Linaro-uefi] [PATCH] MdePkg: ensure SafeString length functions don't access beyond MaxSize
The StrnLenS and AsciiStrnLenS functions, when presented with a string with no terminating NULL in the first MaxSize characters will check the character at String[MaxSize] before checking if Length < MaxSize. (They return the correct value, but have accessed beyond the stated limit in the process.)
Flip the order of the tests to prevent this behaviour.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm leif.lindholm@linaro.org
MdePkg/Library/BaseLib/SafeString.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c index 7c1b075..b0e1ce7 100644 --- a/MdePkg/Library/BaseLib/SafeString.c +++ b/MdePkg/Library/BaseLib/SafeString.c @@ -141,7 +141,7 @@ StrnLenS ( // String then StrnLenS returns MaxSize. At most the first MaxSize characters of String shall // be accessed by StrnLenS. //
- for (Length = 0; (*String != 0) && (Length < MaxSize); String++, Length++) {
- for (Length = 0; (Length < MaxSize) && (*String != 0); String++,
- Length++) { ; } return Length;
@@ -551,7 +551,7 @@ AsciiStrnLenS ( // String then AsciiStrnLenS returns MaxSize. At most the first MaxSize characters of String shall // be accessed by AsciiStrnLenS. //
- for (Length = 0; (*String != 0) && (Length < MaxSize); String++, Length++) {
- for (Length = 0; (Length < MaxSize) && (*String != 0); String++,
- Length++) { ; } return Length;
-- 2.1.4
Linaro-uefi mailing list Linaro-uefi@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-uefi
-
Gao, Liming -
Leif Lindholm -
Yao, Jiewen