Skip to content

Commit

Permalink
fix heap-buffer-overflow on binary paths containing non-ascii characters
Browse files Browse the repository at this point in the history
Before this commit running

   edb --run /home/ivan/Документы/a.out (Документы means "Documents" in Russian)

would cause heap-buffer-overflow in edb:

=================================================================
==26208==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6030004bd19b at pc 0x7f933a072274 bp 0x7ffd5dc26860 sp 0x7ffd5dc26010
WRITE of size 36 at 0x6030004bd19b thread T0
    #0 0x7f933a072273
    eteran#1 0x7f93242f2796 in strcpy bits/string_fortified.h:90
    eteran#2 0x7f93242f2796 in DebuggerCorePlugin::Unix::execute_process(QString const&, QString const&, QList<QByteArray> const&) plugins/DebuggerCore/unix/Unix.cpp:179
    eteran#3 0x7f93241d6dc5 in DebuggerCorePlugin::DebuggerCore::open(QString const&, QString const&, QList<QByteArray> const&, QString const&) plugins/DebuggerCore/unix/linux/DebuggerCore.cpp:925
    eteran#4 0x62b26a in Debugger::commonOpen(QString const&, QList<QByteArray> const&) src/Debugger.cpp:3013
    eteran#5 0x62cdde in Debugger::execute(QString const&, QList<QByteArray> const&) src/Debugger.cpp:3033
    eteran#6 0xc5fa4d in start_debugger src/main.cpp:122
    eteran#7 0xc634c8 in main src/main.cpp:255
    eteran#8 0x7f93371b9b8d in __libc_start_main
    eteran#9 0x4b7879 in _start

0x6030004bd19b is located 0 bytes to the right of 27-byte region [0x6030004bd180,0x6030004bd19b)
allocated by thread T0 here:
    #0 0x7f933a10df40 in operator new[](unsigned long)
    eteran#1 0x7f93242f2239 in DebuggerCorePlugin::Unix::execute_process(QString const&, QString const&, QList<QByteArray> const&) plugins/DebuggerCore/unix/Unix.cpp:178

SUMMARY: AddressSanitizer: heap-buffer-overflow (libasan.so.5+0x52273)
Shadow bytes around the buggy address:
  0x0c068008f9e0: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd
  0x0c068008f9f0: fd fd fa fa fd fd fd fa fa fa fd fd fd fa fa fa
  0x0c068008fa00: 00 00 00 00 fa fa 00 00 00 fa fa fa fd fd fd fa
  0x0c068008fa10: fa fa 00 00 00 fa fa fa fd fd fd fa fa fa 00 00
  0x0c068008fa20: 00 fa fa fa fd fd fd fd fa fa 00 00 00 fa fa fa
=>0x0c068008fa30: 00 00 00[03]fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c068008fa40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c068008fa50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c068008fa60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c068008fa70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c068008fa80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb

The problem was caused by the fact that the buffer size is computed as
a string length in UTF-16 codeunits, but is written with UTF-8 (technically
currect system encoding) codeunits, so its length should equal to
the length of string in UTF-8 codeunits.
  • Loading branch information
sorokin committed Feb 22, 2020
1 parent 5cab4cb commit 6f9d39f
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions plugins/DebuggerCore/unix/Unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ constexpr Exception Exceptions[] = {
#endif
};

char *copyString(QByteArray const &str) {
char *p = new char[str.length() + 1];
std::strcpy(p, str.constData());
return p;
}

}

/**
Expand Down Expand Up @@ -175,15 +181,10 @@ Status Unix::execute_process(const QString &path, const QString &cwd, const QLis

char **p = argv_pointers;

*p = new char[path.length() + 1];
std::strcpy(*p, qPrintable(path));
++p;
*p++ = copyString(path.toLocal8Bit());

for (int i = 0; i < args.count(); ++i) {
const QByteArray s(args[i]);
*p = new char[s.length() + 1];
std::strcpy(*p, s.constData());
++p;
*p++ = copyString(args[i]);
}

*p = nullptr;
Expand Down

0 comments on commit 6f9d39f

Please sign in to comment.