Brad Bishop | 37a0e4d | 2017-12-04 01:01:44 -0500 | [diff] [blame^] | 1 | From 310e2cdc0a46ef62602097f5c21c393571e76df4 Mon Sep 17 00:00:00 2001 |
| 2 | From: Nick Clifton <nickc@redhat.com> |
| 3 | Date: Mon, 13 Feb 2017 14:03:22 +0000 |
| 4 | Subject: [PATCH 2/2] Fix read-after-free error in readelf when processing |
| 5 | multiple, relocated sections in an MSP430 binary. |
| 6 | |
| 7 | PR binutils/21139 |
| 8 | * readelf.c (target_specific_reloc_handling): Add num_syms |
| 9 | parameter. Check for symbol table overflow before accessing |
| 10 | symbol value. If reloc pointer is NULL, discard all saved state. |
| 11 | (apply_relocations): Pass num_syms to target_specific_reloc_handling. |
| 12 | Call target_specific_reloc_handling with a NULL reloc pointer |
| 13 | after processing all of the relocs. |
| 14 | |
| 15 | (cherry pick from commit f84ce13b6708801ca1d6289b7c4003e2f5a6d7f9) |
| 16 | Upstream-Status: Backport [master] |
| 17 | CVE: CVE-2017-6966 |
| 18 | |
| 19 | Signed-off-by: Yuanjie Huang <yuanjie.huang@windriver.com> |
| 20 | --- |
| 21 | binutils/ChangeLog | 10 +++++ |
| 22 | binutils/readelf.c | 109 +++++++++++++++++++++++++++++++++++++++++------------ |
| 23 | 2 files changed, 94 insertions(+), 25 deletions(-) |
| 24 | |
| 25 | diff --git a/binutils/ChangeLog b/binutils/ChangeLog |
| 26 | index 154b797a29..aef0a51f19 100644 |
| 27 | --- a/binutils/ChangeLog |
| 28 | +++ b/binutils/ChangeLog |
| 29 | @@ -1,5 +1,15 @@ |
| 30 | 2017-02-13 Nick Clifton <nickc@redhat.com> |
| 31 | |
| 32 | + PR binutils/21139 |
| 33 | + * readelf.c (target_specific_reloc_handling): Add num_syms |
| 34 | + parameter. Check for symbol table overflow before accessing |
| 35 | + symbol value. If reloc pointer is NULL, discard all saved state. |
| 36 | + (apply_relocations): Pass num_syms to target_specific_reloc_handling. |
| 37 | + Call target_specific_reloc_handling with a NULL reloc pointer |
| 38 | + after processing all of the relocs. |
| 39 | + |
| 40 | +2017-02-13 Nick Clifton <nickc@redhat.com> |
| 41 | + |
| 42 | PR binutils/21137 |
| 43 | * readelf.c (target_specific_reloc_handling): Add end parameter. |
| 44 | Check for buffer overflow before writing relocated values. |
| 45 | diff --git a/binutils/readelf.c b/binutils/readelf.c |
| 46 | index 220671f76f..2b6cef1638 100644 |
| 47 | --- a/binutils/readelf.c |
| 48 | +++ b/binutils/readelf.c |
| 49 | @@ -11340,15 +11340,27 @@ process_syminfo (FILE * file ATTRIBUTE_UNUSED) |
| 50 | |
| 51 | /* Check to see if the given reloc needs to be handled in a target specific |
| 52 | manner. If so then process the reloc and return TRUE otherwise return |
| 53 | - FALSE. */ |
| 54 | + FALSE. |
| 55 | + |
| 56 | + If called with reloc == NULL, then this is a signal that reloc processing |
| 57 | + for the current section has finished, and any saved state should be |
| 58 | + discarded. */ |
| 59 | |
| 60 | static bfd_boolean |
| 61 | target_specific_reloc_handling (Elf_Internal_Rela * reloc, |
| 62 | unsigned char * start, |
| 63 | unsigned char * end, |
| 64 | - Elf_Internal_Sym * symtab) |
| 65 | + Elf_Internal_Sym * symtab, |
| 66 | + unsigned long num_syms) |
| 67 | { |
| 68 | - unsigned int reloc_type = get_reloc_type (reloc->r_info); |
| 69 | + unsigned int reloc_type = 0; |
| 70 | + unsigned long sym_index = 0; |
| 71 | + |
| 72 | + if (reloc) |
| 73 | + { |
| 74 | + reloc_type = get_reloc_type (reloc->r_info); |
| 75 | + sym_index = get_reloc_symindex (reloc->r_info); |
| 76 | + } |
| 77 | |
| 78 | switch (elf_header.e_machine) |
| 79 | { |
| 80 | @@ -11357,13 +11369,24 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, |
| 81 | { |
| 82 | static Elf_Internal_Sym * saved_sym = NULL; |
| 83 | |
| 84 | + if (reloc == NULL) |
| 85 | + { |
| 86 | + saved_sym = NULL; |
| 87 | + return TRUE; |
| 88 | + } |
| 89 | + |
| 90 | switch (reloc_type) |
| 91 | { |
| 92 | case 10: /* R_MSP430_SYM_DIFF */ |
| 93 | if (uses_msp430x_relocs ()) |
| 94 | break; |
| 95 | case 21: /* R_MSP430X_SYM_DIFF */ |
| 96 | - saved_sym = symtab + get_reloc_symindex (reloc->r_info); |
| 97 | + /* PR 21139. */ |
| 98 | + if (sym_index >= num_syms) |
| 99 | + error (_("MSP430 SYM_DIFF reloc contains invalid symbol index %lu\n"), |
| 100 | + sym_index); |
| 101 | + else |
| 102 | + saved_sym = symtab + sym_index; |
| 103 | return TRUE; |
| 104 | |
| 105 | case 1: /* R_MSP430_32 or R_MSP430_ABS32 */ |
| 106 | @@ -11388,16 +11411,21 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, |
| 107 | int reloc_size = reloc_type == 1 ? 4 : 2; |
| 108 | bfd_vma value; |
| 109 | |
| 110 | - value = reloc->r_addend |
| 111 | - + (symtab[get_reloc_symindex (reloc->r_info)].st_value |
| 112 | - - saved_sym->st_value); |
| 113 | - |
| 114 | - if (start + reloc->r_offset + reloc_size >= end) |
| 115 | - /* PR 21137 */ |
| 116 | - error (_("MSP430 sym diff reloc writes past end of section (%p vs %p)\n"), |
| 117 | - start + reloc->r_offset + reloc_size, end); |
| 118 | + if (sym_index >= num_syms) |
| 119 | + error (_("MSP430 reloc contains invalid symbol index %lu\n"), |
| 120 | + sym_index); |
| 121 | else |
| 122 | - byte_put (start + reloc->r_offset, value, reloc_size); |
| 123 | + { |
| 124 | + value = reloc->r_addend + (symtab[sym_index].st_value |
| 125 | + - saved_sym->st_value); |
| 126 | + |
| 127 | + if (start + reloc->r_offset + reloc_size >= end) |
| 128 | + /* PR 21137 */ |
| 129 | + error (_("MSP430 sym diff reloc writes past end of section (%p vs %p)\n"), |
| 130 | + start + reloc->r_offset + reloc_size, end); |
| 131 | + else |
| 132 | + byte_put (start + reloc->r_offset, value, reloc_size); |
| 133 | + } |
| 134 | |
| 135 | saved_sym = NULL; |
| 136 | return TRUE; |
| 137 | @@ -11417,13 +11445,24 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, |
| 138 | { |
| 139 | static Elf_Internal_Sym * saved_sym = NULL; |
| 140 | |
| 141 | + if (reloc == NULL) |
| 142 | + { |
| 143 | + saved_sym = NULL; |
| 144 | + return TRUE; |
| 145 | + } |
| 146 | + |
| 147 | switch (reloc_type) |
| 148 | { |
| 149 | case 34: /* R_MN10300_ALIGN */ |
| 150 | return TRUE; |
| 151 | case 33: /* R_MN10300_SYM_DIFF */ |
| 152 | - saved_sym = symtab + get_reloc_symindex (reloc->r_info); |
| 153 | + if (sym_index >= num_syms) |
| 154 | + error (_("MN10300_SYM_DIFF reloc contains invalid symbol index %lu\n"), |
| 155 | + sym_index); |
| 156 | + else |
| 157 | + saved_sym = symtab + sym_index; |
| 158 | return TRUE; |
| 159 | + |
| 160 | case 1: /* R_MN10300_32 */ |
| 161 | case 2: /* R_MN10300_16 */ |
| 162 | if (saved_sym != NULL) |
| 163 | @@ -11431,15 +11470,20 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, |
| 164 | int reloc_size = reloc_type == 1 ? 4 : 2; |
| 165 | bfd_vma value; |
| 166 | |
| 167 | - value = reloc->r_addend |
| 168 | - + (symtab[get_reloc_symindex (reloc->r_info)].st_value |
| 169 | - - saved_sym->st_value); |
| 170 | - |
| 171 | - if (start + reloc->r_offset + reloc_size >= end) |
| 172 | - error (_("MN10300 sym diff reloc writes past end of section (%p vs %p)\n"), |
| 173 | - start + reloc->r_offset + reloc_size, end); |
| 174 | + if (sym_index >= num_syms) |
| 175 | + error (_("MN10300 reloc contains invalid symbol index %lu\n"), |
| 176 | + sym_index); |
| 177 | else |
| 178 | - byte_put (start + reloc->r_offset, value, reloc_size); |
| 179 | + { |
| 180 | + value = reloc->r_addend + (symtab[sym_index].st_value |
| 181 | + - saved_sym->st_value); |
| 182 | + |
| 183 | + if (start + reloc->r_offset + reloc_size >= end) |
| 184 | + error (_("MN10300 sym diff reloc writes past end of section (%p vs %p)\n"), |
| 185 | + start + reloc->r_offset + reloc_size, end); |
| 186 | + else |
| 187 | + byte_put (start + reloc->r_offset, value, reloc_size); |
| 188 | + } |
| 189 | |
| 190 | saved_sym = NULL; |
| 191 | return TRUE; |
| 192 | @@ -11459,12 +11503,24 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc, |
| 193 | static bfd_vma saved_sym2 = 0; |
| 194 | static bfd_vma value; |
| 195 | |
| 196 | + if (reloc == NULL) |
| 197 | + { |
| 198 | + saved_sym1 = saved_sym2 = 0; |
| 199 | + return TRUE; |
| 200 | + } |
| 201 | + |
| 202 | switch (reloc_type) |
| 203 | { |
| 204 | case 0x80: /* R_RL78_SYM. */ |
| 205 | saved_sym1 = saved_sym2; |
| 206 | - saved_sym2 = symtab[get_reloc_symindex (reloc->r_info)].st_value; |
| 207 | - saved_sym2 += reloc->r_addend; |
| 208 | + if (sym_index >= num_syms) |
| 209 | + error (_("RL78_SYM reloc contains invalid symbol index %lu\n"), |
| 210 | + sym_index); |
| 211 | + else |
| 212 | + { |
| 213 | + saved_sym2 = symtab[sym_index].st_value; |
| 214 | + saved_sym2 += reloc->r_addend; |
| 215 | + } |
| 216 | return TRUE; |
| 217 | |
| 218 | case 0x83: /* R_RL78_OPsub. */ |
| 219 | @@ -12094,7 +12150,7 @@ apply_relocations (void * file, |
| 220 | |
| 221 | reloc_type = get_reloc_type (rp->r_info); |
| 222 | |
| 223 | - if (target_specific_reloc_handling (rp, start, end, symtab)) |
| 224 | + if (target_specific_reloc_handling (rp, start, end, symtab, num_syms)) |
| 225 | continue; |
| 226 | else if (is_none_reloc (reloc_type)) |
| 227 | continue; |
| 228 | @@ -12190,6 +12246,9 @@ apply_relocations (void * file, |
| 229 | } |
| 230 | |
| 231 | free (symtab); |
| 232 | + /* Let the target specific reloc processing code know that |
| 233 | + we have finished with these relocs. */ |
| 234 | + target_specific_reloc_handling (NULL, NULL, NULL, NULL, 0); |
| 235 | |
| 236 | if (relocs_return) |
| 237 | { |
| 238 | -- |
| 239 | 2.11.0 |
| 240 | |