2

I am poking around in an old & quite buggy C program. When compiled with gcc -fsanitize=address I got this error while running the program itself:

==635==ERROR: AddressSanitizer: strcpy-param-overlap: memory ranges [0x7f37e8cfd5b5,0x7f37e8cfd5b8) and [0x7f37e8cfd5b5, 0x7f37e8cfd5b8) overlap
    #0 0x7f390c3a8552 in __interceptor_strcpy /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cc:429
    #1 0x56488e5c1a08 in backupExon src/BackupGenes.c:72
    #2 0x56488e5c2df1 in backupGene src/BackupGenes.c:134
    #3 0x56488e5c426e in BackupArrayD src/BackupGenes.c:227
    #4 0x56488e5c0bb1 in main src/geneid.c:583
    #5 0x7f390b6bfee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2)
    #6 0x56488e5bf46d in _start (/home/darked89/proj_soft/geneidc/crg_github/geneidc/bin/geneid+0x1c46d)

0x7f37e8cfd5b5 is located 3874229 bytes inside of 37337552-byte region [0x7f37e894b800,0x7f37eace71d0)
allocated by thread T0 here:
    #0 0x7f390c41bce8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x56488e618728 in RequestMemoryDumpster src/RequestMemory.c:801
    #2 0x56488e5bfcea in main src/geneid.c:305
    #3 0x7f390b6bfee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2)

The error was caused by this line:

/* backupExon src/BackupGenes.c:65 */
strcpy(d->dumpSites[d->ndumpSites].subtype, E->Acceptor->subtype);

I have replaced it with:

memmove(d->dumpSites[d->ndumpSites].subtype, E->Acceptor->subtype, 
strlen(d->dumpSites[d->ndumpSites].subtype));

The error went away and the program output produced with 2 different data inputs is identical to the results obtained before the change. BTW, more of strcpy bugs remain further down in the source. I need a confirmation that this is the way to fix it.

The issue & the rest of the code is here: https://github.com/darked89/geneidc/issues/2

darked89
  • 332
  • 1
  • 2
  • 17
  • `[0x7fd8e204c5b5,0x7fd8e204c5b8) and [0x7fd8e204c5b5, 0x7fd8e204c5b8)` - these are the same addresses! Are you copying something to itself? – KamilCuk Oct 03 '19 at 10:53
  • 2
    @KamilCuk pretty sure OP is already aware of that. – Marco Bonelli Oct 03 '19 at 10:54
  • @KamilCuk I am trying to refactor old code written long time ago by others. Rewriting it from scratch without the big cleanup is not feasible. – darked89 Oct 03 '19 at 11:19

1 Answers1

1

Assuming that E->Acceptor->subtype is at least as long as d->dumpSites[d->ndumpSites].subtype then there's no problem. You might want to check that first if you didn't already. Actually, you need a +1 to also copy the string terminator (\0), thanks @R.. for spotting it.

Your previous code was making a different assumption: it was assuming that d->dumpSites[d->ndumpSites].subtype was at least as long as E->Acceptor->subtype (the opposite basically).

The real equivalent would be:

memmove(
    d->dumpSites[d->ndumpSites].subtype,
    E->Acceptor->subtype,
    strlen(E->Acceptor->subtype) + 1
);

This is the correct way to fix the code to allow overlapping.

Marco Bonelli
  • 63,369
  • 21
  • 118
  • 128