Hi Shahriyar,
On 13/08/2026 18:28, Shahriyar Jalayeri wrote:
> 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.
>
> The httpc_connection_t passed to httpc_get_file_dns() has the same lifetime
> problem. lwIP stores a pointer to it in httpc_state_t.conn_settings rather
> than a copy, and dereferences that pointer later for the result and
> headers-done callbacks, so an abandoned request reads its result_fn and
> headers_done_fn from a stack frame that is gone.
>
> Allocate the context on the heap, move the connection settings into it
> (along with the TLS allocator the settings point at, so nothing lwIP
> retains points back at the stack), 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.
The problem I see is that this relies on a later callback to reclaim the
heap context, but that callback is not guaranteed to run, especially once
the command removes the netif and stops the device. That fixes the UAF by
extending the lifetime, but can turn Ctrl-C into a leak.
I suggest cancelling the in-flight lwIP operation before returning from
wget, while the stack objects are still valid. That likely means adding a
small DNS callback cancellation helper and an httpc_abort() helper, then
calling it on Ctrl-C and freeing the TLS config before returning.
Thanks,
--
Jerome
> Fixes: 3c656c928bd7 ("net: lwip: add wget command")
> Signed-off-by: Shahriyar Jalayeri <[email protected]>
> ---
> net/lwip/wget.c | 95
> ++++++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 67 insertions(+), 28 deletions(-)
>
> diff --git a/net/lwip/wget.c b/net/lwip/wget.c
> index 9e93765926d..7359094f6be 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,11 @@ struct wget_ctx {
> ulong content_len;
> ulong hash_count;
> enum done_state done;
> + bool abandoned;
> + httpc_connection_t conn;
> +#if CONFIG_IS_ENABLED(WGET_HTTPS)
> + altcp_allocator_t tls_allocator;
> +#endif
> };
>
> static void wget_lwip_fill_info(struct pbuf *hdr, u16_t hdr_len, u32_t
> hdr_cont_len)
> @@ -200,6 +206,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 +240,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 +301,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)
> @@ -299,10 +321,6 @@ static err_t httpc_headers_done_cb(httpc_state_t
> *connection, void *arg, struct
> static int wget_handle_request(struct wget_ctx *ctx, bool is_https,
> struct udevice *udev, struct netif *netif)
> {
> -#if CONFIG_IS_ENABLED(WGET_HTTPS)
> - altcp_allocator_t tls_allocator;
> -#endif
> - httpc_connection_t conn;
> httpc_state_t *state;
> int ret;
>
> @@ -313,7 +331,7 @@ static int wget_handle_request(struct wget_ctx *ctx, bool
> is_https,
> return ret;
> }
>
> - memset(&conn, 0, sizeof(conn));
> + memset(&ctx->conn, 0, sizeof(ctx->conn));
> #if CONFIG_IS_ENABLED(WGET_HTTPS)
> if (is_https) {
> char *ca;
> @@ -351,23 +369,23 @@ static int wget_handle_request(struct wget_ctx *ctx,
> bool is_https,
> printf("WARNING: no CA certificates, ");
> printf("HTTPS connections not authenticated\n");
> }
> - tls_allocator.alloc = &altcp_tls_alloc;
> - tls_allocator.arg =
> + ctx->tls_allocator.alloc = &altcp_tls_alloc;
> + ctx->tls_allocator.arg =
> altcp_tls_create_config_client(ca, ca_sz,
> ctx->server_name);
>
> - if (!tls_allocator.arg) {
> + if (!ctx->tls_allocator.arg) {
> log_err("error: Cannot create a TLS connection\n");
> return -ENODEV;
> }
>
> - conn.altcp_allocator = &tls_allocator;
> + ctx->conn.altcp_allocator = &ctx->tls_allocator;
> }
> #endif
>
> - conn.result_fn = httpc_result_cb;
> - conn.headers_done_fn = httpc_headers_done_cb;
> - if (httpc_get_file_dns(ctx->server_name, ctx->port, ctx->path, &conn,
> + ctx->conn.result_fn = httpc_result_cb;
> + ctx->conn.headers_done_fn = httpc_headers_done_cb;
> + if (httpc_get_file_dns(ctx->server_name, ctx->port, ctx->path,
> &ctx->conn,
> httpc_recv_cb, ctx, &state)) {
> return -ENODEV;
> }
> @@ -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;
> }
>
>
