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.
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;
}
--
2.43.0
