Comment 12 for bug 1011961

Revision history for this message
In , Jonathan Thomas (echidnaman) wrote :

Created attachment 71857
Updated patch

I'm not a kde-runtime maintainer, but here are my thoughts on the patch. The patch does indeed make things much safer, since the old method, as you mentioned, stores the pointer to the location of the data of a temporary QByteArray. With the old method you definitely aren't guaranteed that nothing will step on that data before it is finished being used within the function.

But while it is much safer, it still passes the address of a temporary to ssh_userauth_kbdint_setanswer(), since calling toUtf8() on a QString returns a temporary (in this case) copy of the string as a byte array, e.g.:

+ if (ssh_userauth_kbdint_setanswer(mSession, i, answer.toUtf8().constData()) < 0) {

Depending on how ssh_userauth_kbdint_setanswer is implemented, it may or may not be "fine" to do this, (It does indeed solve the problem in your testing) but it still does leave the code to maybe failing at least some of the time.

It would be better to make "answer" a QByteArray rather than a QString. That way, when you initially assign it with "answer = info.username.toUtf8()", you have an instance of the byte array that will be kept around for all of the current scope. You can then grab its constData() safely, knowing that the memory there will remain unchanged for the life of "answer".

I've attached an updated version of the patch that reflects these changes.