wget_do_request() keeps the transfer context on the stack and passes its
address to the lwIP httpc callbacks. If the user interrupts the transfer
with Ctrl-C while the host name is still resolving, wget_handle_request()
leaves its receive loop without tearing the connection down. The pending
DNS lookup (which lwIP cannot cancel) later resolves, the connection is
established, and httpc_recv_cb() runs against a stack frame that no longer
exists. It writes attacker-controlled TCP data through store_block() to
map_sysmem(ctx->daddr), with ctx read from reused stack.

Allocate the context on the heap and add an 'abandoned' flag. On Ctrl-C
with a request still in flight, mark it abandoned and hand ownership to
the lwIP callback, which frees the context when the connection finally
tears down; the receive, headers-done and result callbacks return early
so a stale callback neither stores data nor touches wget_info.

Fixes: 3c656c928bd7 ("net: lwip: add wget command")
Signed-off-by: Shahriyar Jalayeri <[email protected]>
---
 net/lwip/wget.c | 71 ++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 55 insertions(+), 16 deletions(-)

diff --git a/net/lwip/wget.c b/net/lwip/wget.c
index 9e93765926d..b81509dff3f 100644
--- a/net/lwip/wget.c
+++ b/net/lwip/wget.c
@@ -12,6 +12,7 @@
 #include <lwip/errno.h>
 #include <lwip/timeouts.h>
 #include <rng.h>
+#include <malloc.h>
 #include <mapmem.h>
 #include <net.h>
 #include <time.h>
@@ -39,6 +40,7 @@ struct wget_ctx {
        ulong content_len;
        ulong hash_count;
        enum done_state done;
+       bool abandoned;
 };
 
 static void wget_lwip_fill_info(struct pbuf *hdr, u16_t hdr_len, u32_t 
hdr_cont_len)
@@ -200,6 +202,13 @@ static err_t httpc_recv_cb(void *arg, struct altcp_pcb 
*pcb, struct pbuf *pbuf,
        if (!pbuf)
                return ERR_BUF;
 
+       /* The caller gave up on this request; drop the connection. */
+       if (ctx->abandoned) {
+               altcp_abort(pcb);
+               pbuf_free(pbuf);
+               return ERR_ABRT;
+       }
+
        if (!ctx->start_time)
                ctx->start_time = get_timer(0);
 
@@ -227,6 +236,12 @@ static void httpc_result_cb(void *arg, httpc_result_t 
httpc_result,
        struct wget_ctx *ctx = arg;
        ulong elapsed;
 
+       /* Last callback for an abandoned request: reclaim ctx and stop. */
+       if (ctx->abandoned) {
+               free(ctx);
+               return;
+       }
+
        wget_info->status_code = (u32)srv_res;
 
        if (err == ERR_BUF) {
@@ -282,6 +297,9 @@ static err_t httpc_headers_done_cb(httpc_state_t 
*connection, void *arg, struct
 {
        struct wget_ctx *ctx = arg;
 
+       if (ctx->abandoned)
+               return ERR_BUF;
+
        wget_lwip_fill_info(hdr, hdr_len, content_len);
 
        if (wget_info->check_buffer_size && (ulong)content_len > 
wget_info->buffer_size)
@@ -376,8 +394,17 @@ static int wget_handle_request(struct wget_ctx *ctx, bool 
is_https,
 
        while (!ctx->done) {
                net_lwip_rx(udev, netif);
-               if (ctrlc())
+               if (ctrlc()) {
+                       /*
+                        * A request may still be in flight (e.g. the name is
+                        * still resolving). Hand ctx to the lwIP callback, 
which
+                        * frees it once the connection tears down, instead of
+                        * freeing it here under a live callback.
+                        */
+                       if (!ctx->done)
+                               ctx->abandoned = true;
                        break;
+               }
        }
 
        if (ctx->done == SUCCESS)
@@ -392,27 +419,26 @@ static int wget_handle_request(struct wget_ctx *ctx, bool 
is_https,
 int wget_do_request(ulong dst_addr, char *uri)
 {
        struct udevice *udev;
-       struct wget_ctx ctx;
+       struct wget_ctx *ctx;
        struct netif *netif;
-       bool is_https;
+       bool is_https, abandoned;
        int ret;
 
-       ctx.daddr = dst_addr;
-       ctx.saved_daddr = dst_addr;
-       ctx.done = NOT_DONE;
-       ctx.size = 0;
-       ctx.prevsize = 0;
-       ctx.start_time = 0;
-       ctx.content_len = 0;
-       ctx.hash_count = 0;
+       ctx = calloc(1, sizeof(*ctx));
+       if (!ctx)
+               return -ENOMEM;
+
+       ctx->daddr = dst_addr;
+       ctx->saved_daddr = dst_addr;
+       ctx->done = NOT_DONE;
 
-       ret = parse_url(uri, ctx.server_name, &ctx.port, &ctx.path, &is_https);
+       ret = parse_url(uri, ctx->server_name, &ctx->port, &ctx->path, 
&is_https);
        if (ret)
-               return ret;
+               goto out;
 
        ret = net_lwip_eth_start();
        if (ret)
-               return ret;
+               goto out;
 
        if (!wget_info)
                wget_info = &default_wget_info;
@@ -422,14 +448,27 @@ int wget_do_request(ulong dst_addr, char *uri)
        netif = net_lwip_new_netif(udev);
        if (!netif) {
                net_lwip_eth_stop();
-               return -ENODEV;
+               ret = -ENODEV;
+               goto out;
        }
 
-       ret = wget_handle_request(&ctx, is_https, udev, netif);
+       ret = wget_handle_request(ctx, is_https, udev, netif);
+
+       /*
+        * If the request was abandoned the lwIP callback still owns ctx and
+        * frees it when the connection tears down; do not free it here.
+        */
+       abandoned = ctx->abandoned;
 
        net_lwip_remove_netif(netif);
        net_lwip_eth_stop();
 
+       if (!abandoned)
+               free(ctx);
+
+       return ret;
+out:
+       free(ctx);
        return ret;
 }
 

-- 
2.43.0

Reply via email to