Hi Shahriar,
On 11/08/2026 20:48, 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.
>
> 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;
One more object seems to have the same lifetime issue here: httpc_connection_t
conn is
stack-allocated in wget_handle_request(), but httpc_get_file_dns() keeps a
pointer to it
in the HTTP client state and later dereferences it for the headers/result
callbacks.
So if Ctrl-C makes wget_handle_request() return while DNS/connect is still
pending, ctx
remains valid but conn does not. Could conn be moved into struct wget_ctx (or
otherwise
made to live as long as the request) as well?
> 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;
> }
Thanks,
--
Jerome
