From 04877847ade4ac9216e9f408fd544ade8f90cf9a Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Fri, 14 Jun 2013 16:39:36 +0100 Subject: libelf: check nul-terminated strings properly It is not safe to simply take pointers into the ELF and use them as C pointers. They might not be properly nul-terminated (and the pointers might be wild). So we are going to introduce a new function elf_strval for safely getting strings. This will check that the addresses are in range and that there is a proper nul-terminated string. Of course it might discover that there isn't. In that case, it will be made to fail. This means that elf_note_name might fail, too. For the benefit of call sites which are just going to pass the value to a printf-like function, we provide elf_strfmt which returns "(invalid)" on failure rather than NULL. In this patch we introduce dummy definitions of these functions. We introduce calls to elf_strval and elf_strfmt everywhere, and update all the call sites with appropriate error checking. There is not yet any semantic change, since before this patch all the places where we introduce elf_strval dereferenced the value anyway, so it mustn't have been NULL. In future patches, when elf_strval is made able return NULL, when it does so it will mark the elf "broken" so that an appropriate diagnostic can be printed. This is part of the fix to a security issue, XSA-55. Signed-off-by: Ian Jackson Acked-by: Ian Campbell Reviewed-by: Konrad Rzeszutek Wilk Reviewed-by: Chuck Anderson Reviewed-by: Andrew Cooper v7: Change readnotes.c check to use two if statements rather than ||. v2: Fix coding style, in one "if" statement. --- tools/xcutils/readnotes.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'tools/xcutils') diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c index 7ff2530ed2..cfae99486c 100644 --- a/tools/xcutils/readnotes.c +++ b/tools/xcutils/readnotes.c @@ -63,7 +63,7 @@ struct setup_header { static void print_string_note(const char *prefix, struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note) { - printf("%s: %s\n", prefix, (char*)elf_note_desc(elf, note)); + printf("%s: %s\n", prefix, elf_strfmt(elf, elf_note_desc(elf, note))); } static void print_numeric_note(const char *prefix, struct elf_binary *elf, @@ -103,10 +103,14 @@ static int print_notes(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) start, { ELF_HANDLE_DECL(elf_note) note; int notes_found = 0; + const char *this_note_name; for ( note = start; ELF_HANDLE_PTRVAL(note) < ELF_HANDLE_PTRVAL(end); note = elf_note_next(elf, note) ) { - if (0 != strcmp(elf_note_name(elf, note), "Xen")) + this_note_name = elf_note_name(elf, note); + if (NULL == this_note_name) + continue; + if (0 != strcmp(this_note_name, "Xen")) continue; notes_found++; @@ -294,7 +298,8 @@ int main(int argc, char **argv) shdr = elf_shdr_by_name(&elf, "__xen_guest"); if (ELF_HANDLE_VALID(shdr)) - printf("__xen_guest: %s\n", (char*)elf_section_start(&elf, shdr)); + printf("__xen_guest: %s\n", + elf_strfmt(&elf, elf_section_start(&elf, shdr))); return 0; } -- cgit v1.2.3