Bug 25319

Summary: Usage of unitialized heap in tic4x_print_cond
Product: binutils Reporter: Nguyễn Đức Mạnh <v.manhnd>
Component: binutilsAssignee: Alan Modra <amodra>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: unspecified   
Target Milestone: 2.34   
Host: Target:
Build: Last reconfirmed:
Attachments: PoC

Description Nguyễn Đức Mạnh 2019-12-27 08:12:47 UTC
Created attachment 12151 [details]
PoC

Hello,
There is a usage of uninitialized heap in opcodes/tic4x-dis.c:tic4x_print_cond()

## Analysis
Look at the initialization code of tic4x_conds:
--------------------------------------------
/* Define conditional branch/load suffixes.  Put desired form for
   disassembler last.  */
static const tic4x_cond_t tic4x_conds[] =
{
  { "u",    0x00 },
  { "c",    0x01 }, { "lo",  0x01 },
  { "ls",   0x02 },
  { "hi",   0x03 },
  { "nc",   0x04 }, { "hs",  0x04 },
  { "z",    0x05 }, { "eq",  0x05 },
  { "nz",   0x06 }, { "ne",  0x06 },
  { "n",    0x07 }, { "l",   0x07 }, { "lt",  0x07 },
  { "le",   0x08 },
  { "p",    0x09 }, { "gt",  0x09 },
  { "nn",   0x0a }, { "ge",  0x0a },
  { "nv",   0x0c },
  { "v",    0x0d },
  { "nuf",  0x0e },
  { "uf",   0x0f },
  { "nlv",  0x10 },
  { "lv",   0x11 },
  { "nluf", 0x12 },
  { "luf",  0x13 },
  { "zuf",  0x14 },
  /* Dummy entry, not included in num_conds.  This
     lets code examine entry i+1 without checking
     if we've run off the end of the table.  */
  { "",      0x0}
};
--------------------------------------------
Here you see the largest cond of a tic4x_cond is 0x14.

Look at the following code of opcodes/tic4x-dis.c:tic4x_print_cond():
--------------------------------------------
272:  static int
273:  tic4x_print_cond (struct disassemble_info *info, unsigned int cond)
274:  {
275:    static tic4x_cond_t **condtable = NULL;
276:    unsigned int i;
277:
278:    if (condtable == NULL)
279:      {
280:        condtable = xmalloc (sizeof (tic4x_cond_t *) * 32);
281:        for (i = 0; i < tic4x_num_conds; i++)
282:  	condtable[tic4x_conds[i].cond] = (tic4x_cond_t *)(tic4x_conds + i);
283:      }
284:    if (cond > 31 || condtable[cond] == NULL)
285:      return 0;
286:    if (info != NULL)
287:      (*info->fprintf_func) (info->stream, "%s", condtable[cond]->name);
288:    return 1;
289:  }
--------------------------------------------
At lines 280, condtable is malloced as an array of 32 elements. At line 382, condtable is intialized with condtable[tic4x_conds[i].cond] = (tic4x_cond_t *)(tic4x_conds + i). Because the largest tic4x_cond.cond is 0x14 = 20, so the element from offset 21 to 31 is not initialized. This can lead to information leak due to the print function at 287.

## Reproduction
--------------------PoC code----------------
#include "sysdep.h"
#include "bfd.h"
#include "dis-asm.h"
#include "disassemble.h"

#include <stdint.h>

#define MAX_TEXT_SIZE 256
#define MAX_PAYLOAD_SIZE 100
int gDebug = 0;

typedef struct
{
    char *buffer;
    size_t pos;
} SFILE;

static int objdump_sprintf (SFILE *f, const char *format, ...)
{
    size_t n;
    va_list args;

    va_start (args, format);
    if (f->pos >= MAX_TEXT_SIZE){
        printf("buffer needs more space\n");
        return 0;
    }
    n = vsnprintf (f->buffer + f->pos, MAX_TEXT_SIZE - f->pos, format, args);
    if (gDebug) {
        vfprintf(stdout, format, args);
    }
    va_end (args);
    f->pos += n;

    return n;
}

static void objdump_print_address (bfd_vma vma, struct disassemble_info *inf)
{
    (*inf->fprintf_func) (inf->stream, "0x%lx", vma);
}

int fuzzTest(const uint8_t *Data, size_t Size) {
    char AssemblyText[MAX_TEXT_SIZE];
    struct disassemble_info disasm_info;
    SFILE s;
    bfd abfd;

    if (Size < 10) {
        // 10 bytes for options
        return 0;
    }

    init_disassemble_info (&disasm_info, stdout, (fprintf_ftype) fprintf);
    disasm_info.fprintf_func = objdump_sprintf;
    disasm_info.print_address_func = objdump_print_address;
    disasm_info.display_endian = disasm_info.endian = BFD_ENDIAN_LITTLE;
    disasm_info.buffer = Data;
    disasm_info.buffer_vma = 0x1000;
    disasm_info.buffer_length = Size-10;
    disasm_info.insn_info_valid = 0;
    s.buffer = AssemblyText;
    s.pos = 0;
    disasm_info.stream = &s;
    disasm_info.bytes_per_line = 0;

    disasm_info.arch = Data[Size-1];
    disasm_info.mach = *((unsigned long *) (Data + Size - 9));
    disasm_info.flavour = Data[Size-10];

    if (bfd_lookup_arch (disasm_info.arch, disasm_info.mach) != NULL) {
        disassembler_ftype disasfunc = disassembler(disasm_info.arch, 0, disasm_info.mach, NULL);
        if (disasfunc != NULL) {
            disassemble_init_for_target(&disasm_info);
            disasfunc(0x1000, &disasm_info);
            disassemble_free_target(&disasm_info);
        }
    }

    return 0;
}

size_t readFile(char* fileName, char* result, int maxSize)
{
    FILE *f = fopen(fileName, "rb");
    if (f == NULL) {
        fprintf(stdout, "Error cannot read file %s\n", fileName);
        exit(1);
    }
    fseek(f, 0, SEEK_END);
    size_t fsize = ftell(f);
    fseek(f, 0, SEEK_SET);

    if (maxSize != -1 && fsize > maxSize) { fsize = maxSize; }
    if (fsize > MAX_PAYLOAD_SIZE) fsize = MAX_PAYLOAD_SIZE;
    size_t read = fread(result, 1, fsize, f);
    fclose(f);
    if (read != fsize) {
        fprintf(stdout, "Error while reading file %s\n", fileName);
        exit(1);
    }
    return fsize;
}


int main(int argc, char* argv[])
{
  size_t n;
  uint8_t Data[MAX_PAYLOAD_SIZE];

  if (argc > 2) if (strcmp(argv[2], "debug") == 0) gDebug = 1;

  //while(__AFL_LOOP(1000)) {
    n = readFile(argv[1], (char*)Data, sizeof(Data));
    fuzzTest(Data, n);
  //}
  return 0;
}
------------------------------------------

-----------------Compilation--------------
Compile binutils in 32 bit and address sanitizer.
./configure --disable-gdb --enable-targets=all CC=gcc CXX=g++ CFLAGS='-m32 -ggdb3 -fsanitize=address -O0' LDFLAGS=-m32 CXXFLAGS='-m32 -O0 -fsanitize=address -ggdb3'
And compile the PoC:
gcc -fsanitize=address -m32 -ggdb3 -O0 -I ./binutils-gdb-gcc-asan-32/include/ -I ./binutils-gdb-gcc-asan-32/bfd/ -I ./binutils-gdb-gcc-asan-32/opcodes/ -c fuzz_disassemble.c -o fuzz_disassemble.o
gcc -fsanitize=address -O0 -m32 -ggdb3 fuzz_disassemble.o -o fuzz_disassemble-32-libfuzzer binutils-gdb-gcc-asan-32/opcodes/libopcodes.a binutils-gdb-gcc-asan-32/bfd/libbfd.a binutils-gdb-gcc-asan-32/libiberty/libiberty.a binutils-gdb-gcc-asan-32/zlib/libz.a

------------------Log Crash----------------
root@manh-ubuntu16:~/fuzz/fuzz_binutils# ./fuzz_disassemble-gcc-asan-32 crash-disassemble 
ASAN:SIGSEGV
=================================================================
==21804==ERROR: AddressSanitizer: SEGV on unknown address 0xbebebebe (pc 0x0829b3d5 bp 0xfff0a758 sp 0xfff0a720 T0)
    #0 0x829b3d4 in tic4x_print_cond /root/fuzz/fuzz_binutils/binutils-gdb-gcc-asan-32/opcodes/tic4x-dis.c:287
    #1 0x829b7e9 in tic4x_print_op /root/fuzz/fuzz_binutils/binutils-gdb-gcc-asan-32/opcodes/tic4x-dis.c:376
    #2 0x829c886 in tic4x_disassemble /root/fuzz/fuzz_binutils/binutils-gdb-gcc-asan-32/opcodes/tic4x-dis.c:731
    #3 0x829ce82 in print_insn_tic4x /root/fuzz/fuzz_binutils/binutils-gdb-gcc-asan-32/opcodes/tic4x-dis.c:774
    #4 0x804a9d5 in fuzzTest /root/fuzz/fuzz_binutils/fuzz_disassemble.c:75
    #5 0x804ad93 in main /root/fuzz/fuzz_binutils/fuzz_disassemble.c:115
    #6 0xf7030636 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x18636)
    #7 0x804a250  (/root/fuzz/fuzz_binutils/fuzz_disassemble-gcc-asan-32+0x804a250)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /root/fuzz/fuzz_binutils/binutils-gdb-gcc-asan-32/opcodes/tic4x-dis.c:287 tic4x_print_cond
==21804==ABORTING


--
Thanks & Regards,
Nguyen Duc Manh
VinCSS (a member of Vingroup)
[E] v.manhnd@vincss.net
[W] www.vincss.net
Comment 1 Sourceware Commits 2019-12-29 11:44:48 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8c5e259235a4e4546910245b170de1e29a711034

commit 8c5e259235a4e4546910245b170de1e29a711034
Author: Alan Modra <amodra@gmail.com>
Date:   Sun Dec 29 12:56:29 2019 +1030

    Usage of unitialized heap in tic4x_print_cond
    
    	PR 25319
    	* tic4x-dis.c (tic4x_print_cond): Init all of condtable.
Comment 2 Alan Modra 2019-12-29 21:55:59 UTC
Fixed.
Comment 3 Sourceware Commits 2019-12-29 23:08:00 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=2c5b6e1a1c406cbe06e2d6f77861764ebd01b9ce

commit 2c5b6e1a1c406cbe06e2d6f77861764ebd01b9ce
Author: Alan Modra <amodra@gmail.com>
Date:   Mon Dec 30 09:19:25 2019 +1030

    Re: Usage of unitialized heap in tic4x_print_cond
    
    	PR 25319
    	* tic4x-dis.c (tic4x_print_cond): Correct order of xcalloc args.