From: "Laszlo Ersek" <lersek@redhat.com>
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>,
Maciej Rabeda <maciej.rabeda@linux.intel.com>,
Siyuan Fu <siyuan.fu@intel.com>
Subject: [PATCH 2/2] NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download
Date: Thu, 9 Jan 2020 00:43:13 +0100 [thread overview]
Message-ID: <20200108234313.28510-3-lersek@redhat.com> (raw)
In-Reply-To: <20200108234313.28510-1-lersek@redhat.com>
When downloading over TLS, each TLS message ("APP packet") is returned as
a (decrypted) fragment table by EFI_TLS_PROTOCOL.ProcessPacket().
The TlsProcessMessage() function in "NetworkPkg/HttpDxe/HttpsSupport.c"
linearizes the fragment table into a single contiguous data block. The
resultant flat data block contains both TLS headers and data.
The HttpsReceive() function parses the actual application data -- in this
case: decrypted HTTP data -- out of the flattened TLS data block, peeling
off the TLS headers.
The HttpResponseWorker() function in "NetworkPkg/HttpDxe/HttpImpl.c"
propagates this HTTP data outwards, implementing the
EFI_HTTP_PROTOCOL.Response() function.
Now consider the following documentation for EFI_HTTP_PROTOCOL.Response(),
quoted from "MdePkg/Include/Protocol/Http.h":
> It is the responsibility of the caller to allocate a buffer for Body and
> specify the size in BodyLength. If the remote host provides a response
> that contains a content body, up to BodyLength bytes will be copied from
> the receive buffer into Body and BodyLength will be updated with the
> amount of bytes received and copied to Body. This allows the client to
> download a large file in chunks instead of into one contiguous block of
> memory.
Note that, if the caller-allocated buffer is larger than the
server-provided chunk, then the transfer length is limited by the latter.
This is in fact the dominant case when downloading a huge file (for which
UefiBootManagerLib allocated a huge contiguous RAM Disk buffer) in small
TLS messages.
For adjusting BodyLength as described above -- i.e., to the application
data chunk that has been extracted from the TLS message --, the
HttpResponseWorker() function employs the following assignment:
HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg->BodyLength);
The (UINT32) cast is motivated by the MIN() requirement -- in
"MdePkg/Include/Base.h" -- that both arguments be of the same type.
"Fragment.Len" (NET_FRAGMENT.Len) has type UINT32, and
"HttpMsg->BodyLength" (EFI_HTTP_MESSAGE.BodyLength) has type UINTN.
Therefore a cast is indeed necessary.
Unfortunately, the cast is done in the wrong direction. Consider the
following circumstances:
- "Fragment.Len" happens to be consistently 16KiB, dictated by the HTTPS
Server's TLS stack,
- the size of the file to download is 4GiB + N*16KiB, where N is a
positive integer.
As the download progresses, each received 16KiB application data chunk
brings the *next* input value of BodyLength closer down to 4GiB. The cast
in MIN() always masks off the high-order bits from the input value of
BodyLength, but this is no problem because the low-order bits are nonzero,
therefore the MIN() always permits progress.
However, once BodyLength reaches 4GiB exactly on input, the MIN()
invocation produces a zero value. HttpResponseWorker() adjusts the output
value of BodyLength to zero, and then passes it to HttpParseMessageBody().
HttpParseMessageBody() (in "NetworkPkg/Library/DxeHttpLib/DxeHttpLib.c")
rejects the zero BodyLength with EFI_INVALID_PARAMETER, which is fully
propagated outwards, and aborts the HTTPS download. HttpBootDxe writes the
message "Error: Unexpected network error" to the UEFI console.
For example, a file with size (4GiB + 197MiB) terminates after downloading
just 197MiB.
Invert the direction of the cast: widen "Fragment.Len" to UINTN.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
NetworkPkg/HttpDxe/HttpImpl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 6b877314bd57..1acbb60d1014 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1348,7 +1348,7 @@ HttpResponseWorker (
//
// Process the received the body packet.
//
- HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg->BodyLength);
+ HttpMsg->BodyLength = MIN ((UINTN) Fragment.Len, HttpMsg->BodyLength);
CopyMem (HttpMsg->Body, Fragment.Bulk, HttpMsg->BodyLength);
--
2.19.1.3.g30247aa5d201
next prev parent reply other threads:[~2020-01-08 23:43 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-08 23:43 [PATCH 0/2] UefiBootManagerLib, HttpDxe: tweaks for large HTTP(S) downloads Laszlo Ersek
2020-01-08 23:43 ` [PATCH 1/2] MdeModulePkg/UefiBootManagerLib: log reserved mem allocation failure Laszlo Ersek
2020-01-09 3:08 ` Ni, Ray
2020-01-09 17:24 ` [edk2-devel] " Philippe Mathieu-Daudé
2020-01-13 1:53 ` Siyuan, Fu
2020-01-13 21:05 ` Laszlo Ersek
2020-01-14 6:55 ` Wu, Hao A
2020-01-08 23:43 ` Laszlo Ersek [this message]
2020-01-09 13:29 ` [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix 32-bit truncation in HTTPS download Philippe Mathieu-Daudé
2020-01-10 1:01 ` Siyuan, Fu
2020-01-10 10:49 ` Laszlo Ersek
2020-01-10 15:31 ` Maciej Rabeda
2020-01-14 10:59 ` [edk2-devel] [PATCH 0/2] UefiBootManagerLib, HttpDxe: tweaks for large HTTP(S) downloads Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200108234313.28510-3-lersek@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox