Re: [PATCH 12/17] libdwfl: Reject very short or really large build-ids.

Re: [PATCH 12/17] libdwfl: Reject very short or really large build-ids.

[Roland McGrath]

[-- Attachment #1: Type: text/plain, Size: 234 bytes --]

Put the limit in a macro.  

As specified, there is no limit on the size of a build ID.  So this is
imposing an implementation limit.  That limit is certainly going to be fine
in practice, but it is a gratuitous arbitrary limit.

Re: [PATCH 12/17] libdwfl: Reject very short or really large build-ids.

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

On Tue, May 26, 2015 at 12:41:52PM -0700, Roland McGrath wrote:
> Put the limit in a macro.  

Done.
 
> As specified, there is no limit on the size of a build ID.  So this is
> imposing an implementation limit.  That limit is certainly going to be fine
> in practice, but it is a gratuitous arbitrary limit.

Yeah. Currently all build IDs are a 160 bit (20 byte) hash. I think a
512 bit (64 byte) hash is the maximum anybody will ever realistically use.
It is more likely that the paths the build-id link files are stored at
will change before the number of build ID bytes is increased that much.

Cheers,

Mark

[PATCH 12/17] libdwfl: Reject very short or really large build-ids.

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 3007 bytes --]

We cannot handle build-ids less than at least 3 or more than 64 bytes.
Very big build-ids, or very large debug search paths might have blown
up the stack.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libdwfl/ChangeLog                |  6 ++++++
 libdwfl/dwfl_build_id_find_elf.c | 20 ++++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 080825e..f08200e 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,9 @@
+2015-05-22  Mark Wielaard  <mjw@redhat.com>
+
+	* dwfl_build_id_find_elf.c (__libdwfl_open_by_build_id): Return
+	error when id_len too small or too large. strdup, not strdupa,
+	and free path when done.
+
 2015-05-19  Mark Wielaard  <mjw@redhat.com>
 
 	* elf-from-memory.c (elf_from_remote_memory): Don't allocate all
diff --git a/libdwfl/dwfl_build_id_find_elf.c b/libdwfl/dwfl_build_id_find_elf.c
index 062aad1..ff0e945 100644
--- a/libdwfl/dwfl_build_id_find_elf.c
+++ b/libdwfl/dwfl_build_id_find_elf.c
@@ -1,5 +1,5 @@
 /* Find an ELF file for a module from its build ID.
-   Copyright (C) 2007-2010, 2014 Red Hat, Inc.
+   Copyright (C) 2007-2010, 2014, 2015 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -37,9 +37,17 @@ internal_function
 __libdwfl_open_by_build_id (Dwfl_Module *mod, bool debug, char **file_name,
 			    const size_t id_len, const uint8_t *id)
 {
+  /* We don't handle very short or really large build-ids.  We need at
+     at least 3 and allow for up to 64 (normally ids are 20 long).  */
+  if (id_len < 3 || id_len > 64)
+    {
+      __libdwfl_seterrno (DWFL_E_WRONG_ID_ELF);
+      return -1;
+    }
+
   /* Search debuginfo_path directories' .build-id/ subdirectories.  */
 
-  char id_name[sizeof "/.build-id/" + 1 + id_len * 2 + sizeof ".debug" - 1];
+  char id_name[sizeof "/.build-id/" + 1 + 64 * 2 + sizeof ".debug" - 1];
   strcpy (id_name, "/.build-id/");
   int n = snprintf (&id_name[sizeof "/.build-id/" - 1],
 		    4, "%02" PRIx8 "/", (uint8_t) id[0]);
@@ -55,8 +63,10 @@ __libdwfl_open_by_build_id (Dwfl_Module *mod, bool debug, char **file_name,
 	    ".debug");
 
   const Dwfl_Callbacks *const cb = mod->dwfl->callbacks;
-  char *path = strdupa ((cb->debuginfo_path ? *cb->debuginfo_path : NULL)
-			?: DEFAULT_DEBUGINFO_PATH);
+  char *path = strdup ((cb->debuginfo_path ? *cb->debuginfo_path : NULL)
+		       ?: DEFAULT_DEBUGINFO_PATH);
+  if (path == NULL)
+    return -1;
 
   int fd = -1;
   char *dir;
@@ -90,6 +100,8 @@ __libdwfl_open_by_build_id (Dwfl_Module *mod, bool debug, char **file_name,
       free (name);
     }
 
+  free (path);
+
   /* If we simply found nothing, clear errno.  If we had some other error
      with the file, report that.  Possibly this should treat other errors
      like ENOENT too.  But ignoring all errors could mask some that should
-- 
1.8.3.1