Skip to content

Commit 447354c

Browse files
jenswi-linarojforissier
jenswi-linaro
authored and
jforissier
committed
ldelf: strict checks during relocation
Adds strict check of symbol index, string table index and destination location when relocating an ELF. This fixes an error where a malformed ELF may cause the loader to read/write data from/in other ELF or from the loader itself. Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Reported-by: Martijn Bogaard <martijn@riscure.com> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
1 parent 8dbe2cb commit 447354c

File tree

1 file changed

+47
-27
lines changed

1 file changed

+47
-27
lines changed

‎ldelf/ta_elf_rel.c

+47-27
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,12 @@ static void e32_process_dyn_rel(const Elf32_Sym *sym_tab, size_t num_syms,
131131
size_t name_idx = 0;
132132

133133
sym_idx = ELF32_R_SYM(rel->r_info);
134-
assert(sym_idx < num_syms);
134+
if (sym_idx >= num_syms)
135+
err(TEE_ERROR_GENERIC, "Symbol index out of range");
135136

136137
name_idx = sym_tab[sym_idx].st_name;
137-
assert(name_idx < str_tab_size);
138+
if (name_idx >= str_tab_size)
139+
err(TEE_ERROR_GENERIC, "Name index out of range");
138140
name = str_tab + name_idx;
139141

140142
resolve_sym(name, &val);
@@ -161,15 +163,17 @@ static void e32_relocate(struct ta_elf *elf, unsigned int rel_sidx)
161163
if (sym_tab_idx) {
162164
size_t str_tab_idx = 0;
163165

164-
assert(sym_tab_idx < elf->e_shnum);
166+
if (sym_tab_idx >= elf->e_shnum)
167+
err(TEE_ERROR_GENERIC, "Symtab index out of range");
165168

166169
assert(shdr[sym_tab_idx].sh_entsize == sizeof(Elf32_Sym));
167170

168171
/* Check the address is inside ELF memory */
169172
if (ADD_OVERFLOW(shdr[sym_tab_idx].sh_addr,
170173
shdr[sym_tab_idx].sh_size, &sh_end))
171174
err(TEE_ERROR_SECURITY, "Overflow");
172-
assert(sh_end < (elf->max_addr - elf->load_addr));
175+
if (sh_end >= (elf->max_addr - elf->load_addr))
176+
err(TEE_ERROR_GENERIC, "Symbol table out of range");
173177

174178
sym_tab = (Elf32_Sym *)(elf->load_addr +
175179
shdr[sym_tab_idx].sh_addr);
@@ -182,7 +186,9 @@ static void e32_relocate(struct ta_elf *elf, unsigned int rel_sidx)
182186
if (ADD_OVERFLOW(shdr[str_tab_idx].sh_addr,
183187
shdr[str_tab_idx].sh_size, &sh_end))
184188
err(TEE_ERROR_SECURITY, "Overflow");
185-
assert(sh_end < (elf->max_addr - elf->load_addr));
189+
if (sh_end >= (elf->max_addr - elf->load_addr))
190+
err(TEE_ERROR_GENERIC,
191+
"String table out of range");
186192

187193
str_tab = (const char *)(elf->load_addr +
188194
shdr[str_tab_idx].sh_addr);
@@ -191,27 +197,30 @@ static void e32_relocate(struct ta_elf *elf, unsigned int rel_sidx)
191197
}
192198

193199
/* Check the address is inside TA memory */
194-
assert(shdr[rel_sidx].sh_addr < (elf->max_addr - elf->load_addr));
200+
if (ADD_OVERFLOW(shdr[rel_sidx].sh_addr,
201+
shdr[rel_sidx].sh_size, &sh_end))
202+
err(TEE_ERROR_SECURITY, "Overflow");
203+
if (sh_end >= (elf->max_addr - elf->load_addr))
204+
err(TEE_ERROR_GENERIC, "Relocation table out of range");
195205
rel = (Elf32_Rel *)(elf->load_addr + shdr[rel_sidx].sh_addr);
196206

197-
/* Check the address is inside TA memory */
198-
if (ADD_OVERFLOW(shdr[rel_sidx].sh_addr, shdr[rel_sidx].sh_size,
199-
&sh_end))
200-
err(TEE_ERROR_SECURITY, "Overflow");
201-
assert(sh_end < (elf->max_addr - elf->load_addr));
202207
rel_end = rel + shdr[rel_sidx].sh_size / sizeof(Elf32_Rel);
203208
for (; rel < rel_end; rel++) {
204209
Elf32_Addr *where = NULL;
205210
size_t sym_idx = 0;
206211

207212
/* Check the address is inside TA memory */
208-
assert(rel->r_offset < (elf->max_addr - elf->load_addr));
213+
if (rel->r_offset >= (elf->max_addr - elf->load_addr))
214+
err(TEE_ERROR_GENERIC,
215+
"Relocation offset out of range");
209216
where = (Elf32_Addr *)(elf->load_addr + rel->r_offset);
210217

211218
switch (ELF32_R_TYPE(rel->r_info)) {
212219
case R_ARM_ABS32:
213220
sym_idx = ELF32_R_SYM(rel->r_info);
214-
assert(sym_idx < num_syms);
221+
if (sym_idx >= num_syms)
222+
err(TEE_ERROR_GENERIC,
223+
"Symbol index out of range");
215224
if (sym_tab[sym_idx].st_shndx == SHN_UNDEF) {
216225
/* Symbol is external */
217226
e32_process_dyn_rel(sym_tab, num_syms, str_tab,
@@ -223,7 +232,9 @@ static void e32_relocate(struct ta_elf *elf, unsigned int rel_sidx)
223232
break;
224233
case R_ARM_REL32:
225234
sym_idx = ELF32_R_SYM(rel->r_info);
226-
assert(sym_idx < num_syms);
235+
if (sym_idx >= num_syms)
236+
err(TEE_ERROR_GENERIC,
237+
"Symbol index out of range");
227238
*where += sym_tab[sym_idx].st_value - rel->r_offset;
228239
break;
229240
case R_ARM_RELATIVE:
@@ -252,10 +263,12 @@ static void e64_process_dyn_rela(const Elf64_Sym *sym_tab, size_t num_syms,
252263
size_t name_idx = 0;
253264

254265
sym_idx = ELF64_R_SYM(rela->r_info);
255-
assert(sym_idx < num_syms);
266+
if (sym_idx >= num_syms)
267+
err(TEE_ERROR_GENERIC, "Symbol index out of range");
256268

257269
name_idx = sym_tab[sym_idx].st_name;
258-
assert(name_idx < str_tab_size);
270+
if (name_idx >= str_tab_size)
271+
err(TEE_ERROR_GENERIC, "Name index out of range");
259272
name = str_tab + name_idx;
260273

261274
resolve_sym(name, &val);
@@ -282,15 +295,17 @@ static void e64_relocate(struct ta_elf *elf, unsigned int rel_sidx)
282295
if (sym_tab_idx) {
283296
size_t str_tab_idx = 0;
284297

285-
assert(sym_tab_idx < elf->e_shnum);
298+
if (sym_tab_idx >= elf->e_shnum)
299+
err(TEE_ERROR_GENERIC, "Symtab index out of range");
286300

287301
assert(shdr[sym_tab_idx].sh_entsize == sizeof(Elf64_Sym));
288302

289303
/* Check the address is inside TA memory */
290304
if (ADD_OVERFLOW(shdr[sym_tab_idx].sh_addr,
291305
shdr[sym_tab_idx].sh_size, &sh_end))
292306
err(TEE_ERROR_SECURITY, "Overflow");
293-
assert(sh_end < (elf->max_addr - elf->load_addr));
307+
if (sh_end >= (elf->max_addr - elf->load_addr))
308+
err(TEE_ERROR_GENERIC, "Symbol table out of range");
294309

295310
sym_tab = (Elf64_Sym *)(elf->load_addr +
296311
shdr[sym_tab_idx].sh_addr);
@@ -303,7 +318,9 @@ static void e64_relocate(struct ta_elf *elf, unsigned int rel_sidx)
303318
if (ADD_OVERFLOW(shdr[str_tab_idx].sh_addr,
304319
shdr[str_tab_idx].sh_size, &sh_end))
305320
err(TEE_ERROR_SECURITY, "Overflow");
306-
assert(sh_end < (elf->max_addr - elf->load_addr));
321+
if (sh_end >= (elf->max_addr - elf->load_addr))
322+
err(TEE_ERROR_GENERIC,
323+
"String table out of range");
307324

308325
str_tab = (const char *)(elf->load_addr +
309326
shdr[str_tab_idx].sh_addr);
@@ -312,28 +329,31 @@ static void e64_relocate(struct ta_elf *elf, unsigned int rel_sidx)
312329
}
313330

314331
/* Check the address is inside TA memory */
315-
assert(shdr[rel_sidx].sh_addr < (elf->max_addr - elf->load_addr));
332+
if (ADD_OVERFLOW(shdr[rel_sidx].sh_addr,
333+
shdr[rel_sidx].sh_size, &sh_end))
334+
err(TEE_ERROR_SECURITY, "Overflow");
335+
if (sh_end >= (elf->max_addr - elf->load_addr))
336+
err(TEE_ERROR_GENERIC, "Relocation table out of range");
316337
rela = (Elf64_Rela *)(elf->load_addr + shdr[rel_sidx].sh_addr);
317338

318-
/* Check the address is inside TA memory */
319-
if (ADD_OVERFLOW(shdr[rel_sidx].sh_addr, shdr[rel_sidx].sh_size,
320-
&sh_end))
321-
err(TEE_ERROR_SECURITY, "Overflow");
322-
assert(sh_end < (elf->max_addr - elf->load_addr));
323339
rela_end = rela + shdr[rel_sidx].sh_size / sizeof(Elf64_Rela);
324340
for (; rela < rela_end; rela++) {
325341
Elf64_Addr *where = NULL;
326342
size_t sym_idx = 0;
327343

328344
/* Check the address is inside TA memory */
329-
assert(rela->r_offset < (elf->max_addr - elf->load_addr));
345+
if (rela->r_offset >= (elf->max_addr - elf->load_addr))
346+
err(TEE_ERROR_GENERIC,
347+
"Relocation offset out of range");
330348

331349
where = (Elf64_Addr *)(elf->load_addr + rela->r_offset);
332350

333351
switch (ELF64_R_TYPE(rela->r_info)) {
334352
case R_AARCH64_ABS64:
335353
sym_idx = ELF64_R_SYM(rela->r_info);
336-
assert(sym_idx < num_syms);
354+
if (sym_idx >= num_syms)
355+
err(TEE_ERROR_GENERIC,
356+
"Symbol index out of range");
337357
if (sym_tab[sym_idx].st_shndx == SHN_UNDEF) {
338358
/* Symbol is external */
339359
e64_process_dyn_rela(sym_tab, num_syms, str_tab,

0 commit comments

Comments
 (0)