Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Nullify QString data bytes

Tags:

c++

qt

qstring

I use QStrings to store passwords. If to be more precise, I use QStrings to fetch passwords from GUI. The point is that after password usage/appliance I need to nullify (zero) internal QStrings data bytes with password to eliminate it from memory entirely.

Here are my investigations:

  • After QString destruction it's data remains in memory nonzeroed;
  • When I attempt to modify QString to fulfill it with zeroes it triggers copy-on-write idiom and allocates new memory for a modified variant of data. Old data remains untouched. Same happens even if I use QString::data() method. Not really sure why - probably because it returns not raw char * but QChar *;
  • QString::clear(), = "" does actually the same COW as described above.

Q: How can I implement proper QString cleanup to prevent passwords leaks?

like image 426
Nikolai Shalakin Avatar asked Jul 04 '17 19:07

Nikolai Shalakin


2 Answers

I've got two possible ways of bypassing copy-on-write. I've tried them and they seem to work - didn't use the Qt Creator's memory viewer, but the implicitly shared QStrings used in my code both pointed to the same zeroed data afterwards.

Using constData()

As written in the Qt docs for QString::data() method:

For read-only access, constData() is faster because it never causes a deep copy to occur.

So the possible solution looks like this:

QString str = "password";
QString str2 = str;
QChar* chars = const_cast<QChar*>(str.constData());
for (int i = 0; i < str.length(); ++i)
    chars[i] = '0';
// str and str2 are now both zeroed

This is a legitimate use of const_cast since the underlying data is not really const, so no undefined behaviour here.

Using iterators

From the Qt docs for implicit sharing:

An implicitly shared class has control of its internal data. In any member functions that modify its data, it automatically detaches before modifying the data. Notice, however, the special case with container iterators; see Implicit sharing iterator problem.

So let's move to the section describing this iterator problem:

Implicit sharing has another consequence on STL-style iterators: you should avoid copying a container while iterators are active on that container. The iterators point to an internal structure, and if you copy a container you should be very careful with your iterators. E.g.:

QVector<int> a, b;
a.resize(100000); // make a big vector filled with 0.

QVector<int>::iterator i = a.begin();
// WRONG way of using the iterator i:
b = a;
/*
    Now we should be careful with iterator i since it will point to shared data
    If we do *i = 4 then we would change the shared instance (both vectors)
    The behavior differs from STL containers. Avoid doing such things in Qt.
*/

a[0] = 5;
/*
    Container a is now detached from the shared data,
    and even though i was an iterator from the container a, it now works as an iterator in b.
*/

It is my understanding that, based on the above docs fragment, you should be able to exploit this "wrong usage" of iterators to manipulate your original string with iterators as they don't trigger copy-on-write. It's important that you "intercept" the begin() and end() before any copying occurs:

QString str = "password";
QString::iterator itr = str.begin();
QString::iterator nd = str.end();
QString str2 = str;

while (itr != nd)
{
    *itr = '0';
    ++itr;
} // str and str2 still point to the same data and are both zeroed
like image 115
KjMag Avatar answered Oct 07 '22 19:10

KjMag


You have to be aware of all the temporary copies that may be done. If you want to avoid this you must manually erase memory before deallocating each temporal copy. Unfortunately that cannot be done with the standard QString since the implementation is closed.

You can, however, specialise std::basic_string using a custom allocator, which, before deleting can clean up the memory block (see below for an example). You can use this new secure string to manipulate your password instead of a plain char array, if you find it more convenient. I'm not sure is std::basic_string can be specialised with QChar, but if not you can use any of the Unicode characters from C++11 (char16_t, char32_t...) instead if you need other than ANSI support.

Regarding the user interface, I think an option you have is to create your own text input widget, reimplementing the keyPressEvent / keyReleaseEvent to store the typed password into either the secure string or a char array. Also reimplement the paintEvent to display only stars, dots, or whatever other masking character you want. Once the password has been used just clear the array or empty the secure string.


Update: example of secure string

namespace secure {
  template<class T>
  class allocator : public std::allocator<T> {
  public:
    typedef typename std::allocator<T>::pointer pointer;
    typedef typename std::allocator<T>::size_type size_type;

    template<class U>
    struct rebind {
      typedef allocator<U> other;
    };
    allocator() throw() :
      std::allocator<T>() {}
    allocator(const allocator& other) throw() :
      std::allocator<T>(other) {}
    template <class U>
    allocator(const allocator<U>& other) throw() :
      std::allocator<T>(other) {}

    void deallocate(pointer p, size_type num) {
      memset(p, 0, num); // can be replaced by SecureZeroMemory(p, num) on Windows
      std::allocator<T>::deallocate(p, num);
    }
  };

  class string : public std::basic_string<char, std::char_traits<char>, allocator<char>> {
  public:
    string() :
      basic_string() {}

    string(const string& str) :
      basic_string(str.data(), str.length()) {}

    template<class _Elem, class _Traits, class _Ax>
    string(const std::basic_string<_Elem, _Traits, _Ax>& str) :
      basic_string(str.begin(), str.end()) {}

    string(const char* chars) :
      basic_string(chars) {}

    string(const char* chars, size_type sz) :
      basic_string(chars, sz) {}

    template<class _It>
    string(_It a, _It b) :
      basic_string(a, b) {}
  };
}
like image 45
cbuchart Avatar answered Oct 07 '22 19:10

cbuchart