tftp: prevent overwriting reserved memory
This fixes CVE-2018-18439 ("insufficient boundary checks in network
image boot") by using lmb to check for a valid range to store
received blocks.
Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
Acked-by: Joe Hershberger <joe.hershberger@ni.com>
[trini: Always build lib/lmb.o on LMB and lib/fdtdec.o on OF_LIBFDT]
Signed-off-by: Tom Rini <trini@konsulko.com>
diff --git a/lib/Makefile b/lib/Makefile
index f5de4a8..61d7ff0 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -30,14 +30,12 @@
obj-y += crc8.o
obj-y += crc16.o
obj-$(CONFIG_ERRNO_STR) += errno_str.o
-obj-$(CONFIG_OF_LIBFDT) += fdtdec.o
obj-$(CONFIG_FIT) += fdtdec_common.o
obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
obj-$(CONFIG_IMAGE_SPARSE) += image-sparse.o
obj-y += initcall.o
-obj-$(CONFIG_LMB) += lmb.o
obj-y += ldiv.o
obj-$(CONFIG_MD5) += md5.o
obj-y += net_utils.o
@@ -89,9 +87,11 @@
obj-$(CONFIG_CRC32C) += crc32c.o
obj-y += ctype.o
obj-y += div64.o
+obj-$(CONFIG_OF_LIBFDT) += fdtdec.o
obj-y += hang.o
obj-y += linux_compat.o
obj-y += linux_string.o
+obj-$(CONFIG_LMB) += lmb.o
obj-y += membuff.o
obj-$(CONFIG_REGEX) += slre.o
obj-y += string.o
diff --git a/net/tftp.c b/net/tftp.c
index 68ffd81..a9335b1b7 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -17,6 +17,8 @@
#include <flash.h>
#endif
+DECLARE_GLOBAL_DATA_PTR;
+
/* Well known TFTP port # */
#define WELL_KNOWN_PORT 69
/* Millisecs to timeout for lost pkt */
@@ -81,6 +83,10 @@
/* memory offset due to wrapping */
static ulong tftp_block_wrap_offset;
static int tftp_state;
+static ulong tftp_load_addr;
+#ifdef CONFIG_LMB
+static ulong tftp_load_size;
+#endif
#ifdef CONFIG_TFTP_TSIZE
/* The file size reported by the server */
static int tftp_tsize;
@@ -164,10 +170,11 @@
#endif /* CONFIG_MCAST_TFTP */
-static inline void store_block(int block, uchar *src, unsigned len)
+static inline int store_block(int block, uchar *src, unsigned int len)
{
ulong offset = block * tftp_block_size + tftp_block_wrap_offset;
ulong newsize = offset + len;
+ ulong store_addr = tftp_load_addr + offset;
#ifdef CONFIG_SYS_DIRECT_FLASH_TFTP
int i, rc = 0;
@@ -175,24 +182,32 @@
/* start address in flash? */
if (flash_info[i].flash_id == FLASH_UNKNOWN)
continue;
- if (load_addr + offset >= flash_info[i].start[0]) {
+ if (store_addr >= flash_info[i].start[0]) {
rc = 1;
break;
}
}
if (rc) { /* Flash is destination for this packet */
- rc = flash_write((char *)src, (ulong)(load_addr+offset), len);
+ rc = flash_write((char *)src, store_addr, len);
if (rc) {
flash_perror(rc);
- net_set_state(NETLOOP_FAIL);
- return;
+ return rc;
}
} else
#endif /* CONFIG_SYS_DIRECT_FLASH_TFTP */
{
- void *ptr = map_sysmem(load_addr + offset, len);
+ void *ptr;
+#ifdef CONFIG_LMB
+ if (store_addr < tftp_load_addr ||
+ store_addr + len > tftp_load_addr + tftp_load_size) {
+ puts("\nTFTP error: ");
+ puts("trying to overwrite reserved memory...\n");
+ return -1;
+ }
+#endif
+ ptr = map_sysmem(store_addr, len);
memcpy(ptr, src, len);
unmap_sysmem(ptr);
}
@@ -203,6 +218,8 @@
if (net_boot_file_size < newsize)
net_boot_file_size = newsize;
+
+ return 0;
}
/* Clear our state ready for a new transfer */
@@ -606,7 +623,11 @@
timeout_count_max = tftp_timeout_count_max;
net_set_timeout_handler(timeout_ms, tftp_timeout_handler);
- store_block(tftp_cur_block - 1, pkt + 2, len);
+ if (store_block(tftp_cur_block - 1, pkt + 2, len)) {
+ eth_halt();
+ net_set_state(NETLOOP_FAIL);
+ break;
+ }
/*
* Acknowledge the block just received, which will prompt
@@ -695,6 +716,25 @@
}
}
+/* Initialize tftp_load_addr and tftp_load_size from load_addr and lmb */
+static int tftp_init_load_addr(void)
+{
+#ifdef CONFIG_LMB
+ struct lmb lmb;
+ phys_size_t max_size;
+
+ lmb_init_and_reserve(&lmb, gd->bd->bi_dram[0].start,
+ gd->bd->bi_dram[0].size, (void *)gd->fdt_blob);
+
+ max_size = lmb_get_unreserved_size(&lmb, load_addr);
+ if (!max_size)
+ return -1;
+
+ tftp_load_size = max_size;
+#endif
+ tftp_load_addr = load_addr;
+ return 0;
+}
void tftp_start(enum proto_t protocol)
{
@@ -791,7 +831,14 @@
} else
#endif
{
- printf("Load address: 0x%lx\n", load_addr);
+ if (tftp_init_load_addr()) {
+ eth_halt();
+ net_set_state(NETLOOP_FAIL);
+ puts("\nTFTP error: ");
+ puts("trying to overwrite reserved memory...\n");
+ return;
+ }
+ printf("Load address: 0x%lx\n", tftp_load_addr);
puts("Loading: *\b");
tftp_state = STATE_SEND_RRQ;
#ifdef CONFIG_CMD_BOOTEFI
@@ -842,9 +889,15 @@
{
tftp_filename[0] = 0;
+ if (tftp_init_load_addr()) {
+ eth_halt();
+ net_set_state(NETLOOP_FAIL);
+ puts("\nTFTP error: trying to overwrite reserved memory...\n");
+ return;
+ }
printf("Using %s device\n", eth_get_name());
printf("Listening for TFTP transfer on %pI4\n", &net_ip);
- printf("Load address: 0x%lx\n", load_addr);
+ printf("Load address: 0x%lx\n", tftp_load_addr);
puts("Loading: *\b");