[edk2] [PATCH] MdePkg: ensure SafeString length functions don't access beyond MaxSize

[Leif Lindholm]

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 <***@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;

[Yao, Jiewen]

Thank to catch this.

Reviewed by: Yao, Jiewen <***@intel.com>


-----Original Message-----
From: Linaro-uefi [mailto:linaro-uefi-***@lists.linaro.org] On Behalf Of Leif Lindholm
Sent: Friday, July 10, 2015 9:21 PM
To: edk2-***@lists.sourceforge.net
Cc: Kinney, Michael D; Gao, Liming; linaro-***@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 <***@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-***@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-uefi

[Gao, Liming]

Reviewed-by: 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-***@lists.sourceforge.net
Cc: Kinney, Michael D; Gao, Liming; linaro-***@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 <***@intel.com>


-----Original Message-----
From: Linaro-uefi [mailto:linaro-uefi-***@lists.linaro.org] On Behalf Of Leif Lindholm
Sent: Friday, July 10, 2015 9:21 PM
To: edk2-***@lists.sourceforge.net
Cc: Kinney, Michael D; Gao, Liming; linaro-***@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 <***@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-***@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-uefi

[Leif Lindholm]

Sure - please find attached.

Regards,

Leif

Post by Gao, Liming
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
Subject: RE: [Linaro-uefi] [PATCH] MdePkg: ensure SafeString length functions don't access beyond MaxSize
Thank to catch this.
-----Original Message-----
Sent: Friday, July 10, 2015 9:21 PM
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
---
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
https://lists.linaro.org/mailman/listinfo/linaro-uefi