Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elffile clobbers .bss if there is only one loadable segment #25

Open
wumb0 opened this issue Feb 1, 2018 · 0 comments
Open

Elffile clobbers .bss if there is only one loadable segment #25

wumb0 opened this issue Feb 1, 2018 · 0 comments

Comments

@wumb0
Copy link

wumb0 commented Feb 1, 2018

Using the following C program to tell if global (.bss) data is corrupted:

#include <stdio.h>
long global_nums[255];
int main(){
        int i;
        for (i=0;i<255;++i){
                if (global_nums[i] != 0){
                        puts("Corrupted global (.bss)");
                        return -1; 
                }
        }
        puts("No errors.");
        return 0;
}

The binary is compiled with GCC and linked with a custom linker script that only produces a single LOADable section that results in the following segment structure:

root@c0f431c06c51:/patchkit# readelf -l test

Elf file type is EXEC (Executable file)
Entry point 0x80482d0
There are 7 program headers, starting at offset 52

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  PHDR           0x000034 0x08048034 0x08048034 0x000e0 0x000e0 R E 0x4
  INTERP         0x000114 0x08048114 0x08048114 0x00013 0x00013 R   0x1
      [Requesting program interpreter: /lib/ld-linux.so.2]
  LOAD           0x000000 0x08048000 0x08048000 0x006c4 0x00afc RWE 0x1000
  DYNAMIC        0x0005bc 0x080485bc 0x080485bc 0x000e8 0x000e8 RW  0x4
  NOTE           0x000128 0x08048128 0x08048128 0x00044 0x00044 R   0x4
  GNU_EH_FRAME   0x0004b8 0x080484b8 0x080484b8 0x0002c 0x0002c R   0x4
  GNU_STACK      0x000000 0x00000000 0x00000000 0x00000 0x00000 RW  0x10

 Section to Segment mapping:
  Segment Sections...
   00
   01     .interp
   02     .interp .note.ABI-tag .note.gnu.build-id .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rel.got .rel.plt .init .plt .plt.got .text .fini .rodata .eh_frame_hdr .eh_frame .init_array .fini_array .jcr .dynamic .got .got.plt .data .bss
   03     .dynamic
   04     .note.ABI-tag .note.gnu.build-id
   05     .eh_frame_hdr
   06

Test modification via elffile (some ripped from core/binary.py):

from util import elffile
from util.elffile import PT
import sys

align = 0x1000
start = 0xFFFFFFFFFFFFFFFF
end = 0
e = elffile.open(sys.argv[1])
for ph in reversed(e.progs):
    if ph.isload:
        start = min(start, ph.vaddr)
        end = max(ph.vaddr + ph.vsize, end)

addr = end
ph = e.programHeaderClass()
ph.data = bytearray("hello world")
ph.type = PT['PT_LOAD'].code
ph.vaddr = (addr + align - 1) & ~(align - 1)
ph.paddr = ph.vaddr
ph.flags = 7
ph.align = align
ph.memsz = 0
ph.filesz = 0
e.progs.append(ph)
e.save(sys.argv[1]+".mod")

Patching in a new segment with patchkit's elffile implementation (via the script above) results in the error being printed. elffile is writing the program header table over where the .bss would be resulting in corruption of the data when it is loaded into memory (.bss has phdr data in it instead of 0s).
Running test:

root@c0f431c06c51:/patchkit# ./test
No errors.

Running test.mod:

root@c0f431c06c51:/patchkit# ./test.mod
Corrupted global (.bss)

Problem: when the memsz > filesz of a LOAD segment it is unsafe to just append the phdr table to it since the data in the space between (vaddr+filesz) and (vaddr+memsz) is expected to be 0.
Solution: Allocate the slack space manually if memsz > filesz of the first LOADable segment and put the phdr table after it (util/elffile.py line 871-878)

        for p in self.progs:
            if p.offset == 0 and PT[p.type] == PT['PT_LOAD']:
                # HACK: put PHDR at the end of the first segment
                if p.memsz > p.filesz:  # added this if statement
                     # account for .bss
                     p.data.extend(bytes('\x00' * (p.memsz - p.filesz)))
                     p.filesz = p.memsz
                phoff = p.offset + len(p.data)

Patching and running the test binary now results in a clean run.
I think this solution will be fine, but I don't want to mess up the main branch so I am merely suggesting the change here.
My only concern is now a SHT_NOBITS section will have bits in the file... but sections aren't actually needed so I don't think it matters that much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant