smex: harden ELF parsing against malformed input

smex/elf.c

@@ -44,8 +44,29 @@ static int elf_read_sections(struct elf_module *module, bool verbose)
 		return count > 0 ? -ENODATA : -errno;
 	}
 
-	/* read in strings */
-	module->strings = calloc(1, section[hdr->shstrndx].size);
+	/* the string-table section index comes from the ELF header and is used
+	 * to index section[]; reject an out-of-range value before dereferencing
+	 */
+	if (hdr->shstrndx >= hdr->shnum) {
+		fprintf(stderr, "error: %s invalid shstrndx %u >= shnum %u\n",
+			module->elf_file, hdr->shstrndx, hdr->shnum);
+		return -ENOEXEC;
+	}
+
+	/* a zero-size string section leaves module->strings unusable and would
+	 * break later string lookups; reject it explicitly
+	 */
+	if (section[hdr->shstrndx].size == 0) {
+		fprintf(stderr, "error: %s has zero-size string section\n",
+			module->elf_file);
+		return -ENOEXEC;
+	}
+
+	/* read in strings; allocate one extra byte (calloc zeroes it) so the
+	 * table is always NUL-terminated and string lookups cannot run off the
+	 * end even if the section itself lacks a terminator
+	 */
+	module->strings = calloc(1, section[hdr->shstrndx].size + 1);
 	if (!module->strings) {
 		fprintf(stderr, "error: failed %s to read ELF strings for %d\n",
 			module->elf_file, -errno);
@@ -67,6 +88,18 @@ static int elf_read_sections(struct elf_module *module, bool verbose)
 		return count > 0 ? -ENODATA : -errno;
 	}
 
+	/* every section name is used as an offset into the string table; make
+	 * sure each stays within it so later "module->strings + name" reads
+	 * cannot run past the table
+	 */
+	for (i = 0; i < hdr->shnum; i++) {
+		if (section[i].name >= section[hdr->shstrndx].size) {
+			fprintf(stderr, "error: %s section %d name offset %u out of range\n",
+				module->elf_file, i, section[i].name);
+			return -ENOEXEC;
+		}
+	}
+
 	module->bss_index = elf_find_section(module, ".bss");
 	if (module->bss_index < 0) {
 		fprintf(stderr, "Can't find .bss section in %s",
@@ -392,8 +425,22 @@ int elf_find_section(const struct elf_module *module, const char *name)
 		return -EINVAL;
 	}
 
+	if (hdr->shstrndx >= hdr->shnum) {
+		fprintf(stderr, "error: invalid shstrndx %u >= shnum %u\n",
+			hdr->shstrndx, hdr->shnum);
+		return -EINVAL;
+	}
+
 	section = &module->section[hdr->shstrndx];
 
+	/* a zero-size string section would make the buffer[size - 1]
+	 * NUL-termination below write before the allocation
+	 */
+	if (section->size == 0) {
+		fprintf(stderr, "error: zero-size string section\n");
+		return -EINVAL;
+	}
+
 	/* alloc data data */
 	buffer = calloc(1, section->size);
 	if (!buffer)
@@ -482,6 +529,10 @@ int elf_read_section(const struct elf_module *module, const char *section_name,
 
 error:
 	free(*dst_buff);
+	/* clear the caller's pointer so a caller cleanup path (e.g. ldc.c's
+	 * "if (buffer) free(buffer)") does not free the same buffer again
+	 */
+	*dst_buff = NULL;
 	return ret;
 }
 

smex/ldc.c

@@ -26,6 +26,13 @@ static int fw_version_copy(const struct elf_module *src,
 	if (section_size < 0)
 		return section_size;
 
+	if ((size_t)section_size < sizeof(struct sof_ipc_fw_ready)) {
+		fprintf(stderr, "error: .fw_ready section too small: %d\n",
+			section_size);
+		free(buffer);
+		return -EINVAL;
+	}
+
 	memcpy(&header->version,
 	       &((struct sof_ipc_fw_ready *)buffer)->version,
 	       sizeof(header->version));
@@ -50,13 +57,37 @@ static int fw_version_copy(const struct elf_module *src,
 		return section_size;
 
 	ext_hdr = (struct ext_man_elem_header *)buffer;
-	while ((uintptr_t)ext_hdr < (uintptr_t)buffer + section_size) {
+	while ((uintptr_t)ext_hdr + sizeof(*ext_hdr) <=
+	       (uintptr_t)buffer + section_size) {
 		if (ext_hdr->type == EXT_MAN_ELEM_DBG_ABI) {
+			/* make sure the whole dbg-abi element is within the
+			 * section before reading it
+			 */
+			if (ext_hdr->elem_size < sizeof(struct ext_man_dbg_abi) ||
+			    (uintptr_t)ext_hdr + sizeof(struct ext_man_dbg_abi) >
+			    (uintptr_t)buffer + section_size) {
+				fprintf(stderr, "error: %s truncated dbg-abi element\n",
+					src->elf_file);
+				free(buffer);
+				return -ENOEXEC;
+			}
 			header->version.abi_version =
 				((struct ext_man_dbg_abi *)
 						ext_hdr)->dbg_abi.abi_dbg_version;
 			break;
 		}
+		/* a malformed element size would loop forever (0) or advance
+		 * the cursor past the section; reject the image rather than
+		 * silently stopping
+		 */
+		if (ext_hdr->elem_size == 0 ||
+		    (uintptr_t)ext_hdr + ext_hdr->elem_size >
+		    (uintptr_t)buffer + section_size) {
+			fprintf(stderr, "error: %s malformed ext-manifest element\n",
+				src->elf_file);
+			free(buffer);
+			return -ENOEXEC;
+		}
 		//move to the next entry
 		ext_hdr = (struct ext_man_elem_header *)
 				((uint8_t *)ext_hdr + ext_hdr->elem_size);