* [Patch 1/5 v2] MdeModulePkg/DxeHttpLib: Add boundary condition check.
@ 2017-12-26 2:36 Jiaxin Wu
2017-12-26 4:48 ` Gary Lin
0 siblings, 1 reply; 2+ messages in thread
From: Jiaxin Wu @ 2017-12-26 2:36 UTC (permalink / raw)
To: edk2-devel; +Cc: Gary Lin, Ye Ting, Fu Siyuan, Wang Fan, Wu Jiaxin
v2:
* Fix GCC the build error.
This patch is to add the boundary condition check to make sure
the accessed buffer is valid.
Cc: Gary Lin <glin@suse.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Wang Fan <fan.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 38 +++++++++++++++++++++++++---
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
index caddbb7..915b81d 100644
--- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
+++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
@@ -54,11 +54,11 @@ UriPercentDecode (
Index = 0;
Offset = 0;
HexStr[2] = '\0';
while (Index < BufferLength) {
if (Buffer[Index] == '%') {
- if (!NET_IS_HEX_CHAR (Buffer[Index+1]) || !NET_IS_HEX_CHAR (Buffer[Index+2])) {
+ if (Index + 1 >= BufferLength || Index + 2 >= BufferLength || !NET_IS_HEX_CHAR (Buffer[Index+1]) || !NET_IS_HEX_CHAR (Buffer[Index+2])) {
return EFI_INVALID_PARAMETER;
}
HexStr[0] = Buffer[Index+1];
HexStr[1] = Buffer[Index+2];
ResultBuffer[Offset] = (CHAR8) AsciiStrHexToUintn (HexStr);
@@ -1556,20 +1556,31 @@ HttpGetFieldNameAndValue (
)
{
CHAR8 *FieldNameStr;
CHAR8 *FieldValueStr;
CHAR8 *StrPtr;
+ CHAR8 *EndofHeader;
if (String == NULL || FieldName == NULL || FieldValue == NULL) {
return NULL;
}
*FieldName = NULL;
*FieldValue = NULL;
FieldNameStr = NULL;
FieldValueStr = NULL;
StrPtr = NULL;
+ EndofHeader = NULL;
+
+
+ //
+ // Check whether the raw HTTP header string is valid or not.
+ //
+ EndofHeader = AsciiStrStr (String, "\r\n\r\n");
+ if (EndofHeader == NULL) {
+ return NULL;
+ }
//
// Each header field consists of a name followed by a colon (":") and the field value.
//
FieldNameStr = String;
@@ -1583,17 +1594,36 @@ HttpGetFieldNameAndValue (
//
*(FieldValueStr - 1) = 0;
//
// The field value MAY be preceded by any amount of LWS, though a single SP is preferred.
+ // Note: LWS = [CRLF] 1*(SP|HT), it can be '\r\n ' or '\r\n\t' or ' ' or '\t'.
+ // CRLF = '\r\n'.
+ // SP = ' '.
+ // HT = '\t' (Tab).
//
while (TRUE) {
if (*FieldValueStr == ' ' || *FieldValueStr == '\t') {
+ //
+ // Boundary condition check.
+ //
+ if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 1) {
+ return NULL;
+ }
+
FieldValueStr ++;
- } else if (*FieldValueStr == '\r' && *(FieldValueStr + 1) == '\n' &&
- (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')) {
- FieldValueStr = FieldValueStr + 3;
+ } else if (*FieldValueStr == '\r') {
+ //
+ // Boundary condition check.
+ //
+ if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 3) {
+ return NULL;
+ }
+
+ if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')) {
+ FieldValueStr = FieldValueStr + 3;
+ }
} else {
break;
}
}
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Patch 1/5 v2] MdeModulePkg/DxeHttpLib: Add boundary condition check.
2017-12-26 2:36 [Patch 1/5 v2] MdeModulePkg/DxeHttpLib: Add boundary condition check Jiaxin Wu
@ 2017-12-26 4:48 ` Gary Lin
0 siblings, 0 replies; 2+ messages in thread
From: Gary Lin @ 2017-12-26 4:48 UTC (permalink / raw)
To: Jiaxin Wu; +Cc: edk2-devel, Ye Ting, Wang Fan, Fu Siyuan
On Tue, Dec 26, 2017 at 10:36:47AM +0800, Jiaxin Wu wrote:
> v2:
> * Fix GCC the build error.
>
> This patch is to add the boundary condition check to make sure
> the accessed buffer is valid.
>
> Cc: Gary Lin <glin@suse.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Wang Fan <fan.wang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
> MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c | 38 +++++++++++++++++++++++++---
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> index caddbb7..915b81d 100644
> --- a/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> +++ b/MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c
> @@ -54,11 +54,11 @@ UriPercentDecode (
> Index = 0;
> Offset = 0;
> HexStr[2] = '\0';
> while (Index < BufferLength) {
> if (Buffer[Index] == '%') {
> - if (!NET_IS_HEX_CHAR (Buffer[Index+1]) || !NET_IS_HEX_CHAR (Buffer[Index+2])) {
> + if (Index + 1 >= BufferLength || Index + 2 >= BufferLength || !NET_IS_HEX_CHAR (Buffer[Index+1]) || !NET_IS_HEX_CHAR (Buffer[Index+2])) {
> return EFI_INVALID_PARAMETER;
> }
> HexStr[0] = Buffer[Index+1];
> HexStr[1] = Buffer[Index+2];
> ResultBuffer[Offset] = (CHAR8) AsciiStrHexToUintn (HexStr);
> @@ -1556,20 +1556,31 @@ HttpGetFieldNameAndValue (
> )
> {
> CHAR8 *FieldNameStr;
> CHAR8 *FieldValueStr;
> CHAR8 *StrPtr;
> + CHAR8 *EndofHeader;
>
> if (String == NULL || FieldName == NULL || FieldValue == NULL) {
> return NULL;
> }
>
> *FieldName = NULL;
> *FieldValue = NULL;
> FieldNameStr = NULL;
> FieldValueStr = NULL;
> StrPtr = NULL;
> + EndofHeader = NULL;
> +
> +
> + //
> + // Check whether the raw HTTP header string is valid or not.
> + //
> + EndofHeader = AsciiStrStr (String, "\r\n\r\n");
> + if (EndofHeader == NULL) {
> + return NULL;
> + }
>
> //
> // Each header field consists of a name followed by a colon (":") and the field value.
> //
> FieldNameStr = String;
> @@ -1583,17 +1594,36 @@ HttpGetFieldNameAndValue (
> //
> *(FieldValueStr - 1) = 0;
>
> //
> // The field value MAY be preceded by any amount of LWS, though a single SP is preferred.
> + // Note: LWS = [CRLF] 1*(SP|HT), it can be '\r\n ' or '\r\n\t' or ' ' or '\t'.
> + // CRLF = '\r\n'.
> + // SP = ' '.
> + // HT = '\t' (Tab).
> //
> while (TRUE) {
> if (*FieldValueStr == ' ' || *FieldValueStr == '\t') {
> + //
> + // Boundary condition check.
Trailing whitespaces (nitpicking :-)
> + //
> + if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 1) {
> + return NULL;
ditto
> + }
> +
ditto
> FieldValueStr ++;
> - } else if (*FieldValueStr == '\r' && *(FieldValueStr + 1) == '\n' &&
> - (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')) {
> - FieldValueStr = FieldValueStr + 3;
> + } else if (*FieldValueStr == '\r') {
> + //
> + // Boundary condition check.
ditto
> + //
> + if ((UINTN)EndofHeader - (UINTN)(FieldValueStr) < 3) {
> + return NULL;
ditto
> + }
> +
> + if (*(FieldValueStr + 1) == '\n' && (*(FieldValueStr + 2) == ' ' || *(FieldValueStr + 2) == '\t')) {
> + FieldValueStr = FieldValueStr + 3;
> + }
> } else {
> break;
> }
> }
>
This patch fixes the gcc build.
Tested-by: Gary Lin <glin@suse.com>
Gary Lin
> --
> 1.9.5.msysgit.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-12-26 4:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-26 2:36 [Patch 1/5 v2] MdeModulePkg/DxeHttpLib: Add boundary condition check Jiaxin Wu
2017-12-26 4:48 ` Gary Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox