Insecure Password handling

[Belatronix]

Hi,

i would like to discuss a possible security issue that i see inside of the Nitrokey App.

I will show it at the following piece of code outside of stick20changepassworddialog.cpp on Line 236

---

void DialogChangePassword::Stick10ChangePassword(void) {
int ret;

int password_length;
// this is rubbish - replace all occurences of password_length with the Macro
// STICK10_PASSWORD_LEN
QByteArray PasswordString;
password_length = STICK10_PASSWORD_LEN;
unsigned char old_pin[password_length + 1];
unsigned char new_pin[password_length + 1];

memset(old_pin, 0, password_length + 1);
memset(new_pin, 0, password_length + 1);

-> Here it starts!!!!

PasswordString = ui->lineEdit_OldPW->text().toLatin1();
strncpy((char *)old_pin, PasswordString.data(), password_length);

-> copying the Plain! Password unhashed into the memory.

PasswordString = ui->lineEdit_NewPW_1->text().toLatin1();
strncpy((char *)new_pin, PasswordString.data(), password_length);

-> copying the new Plain! Password also unhashed into the memory
-> since here a possible Attack can fish the old and the new password.

// Change password
if (PasswordKind == STICK10_PASSWORD_KIND_ADMIN)
ret = cryptostick->changeAdminPin(old_pin, new_pin);
else // if ( PasswordKind == STICK10_PASSWORD_KIND_USER )
ret = cryptostick->changeUserPin(old_pin, new_pin);

if (ret == CMD_STATUS_WRONG_PASSWORD) {
csApplet->warningBox(tr("Wrong password."));
} else if (ret != CMD_STATUS_OK) {
csApplet->warningBox(tr("Couldn't change %1 password")
.arg((PasswordKind == STICK10_PASSWORD_KIND_USER) ? "user" : "admin"));
}
}`
-> Amazing! Both of the passwords are still exist in the Memory to get fished
-No deleting or something else-

My suggestions about a possible solution:
1.) The Passwords has to be cleaned out of the memory urgently after use. In this Case the method cryptostick->changeAdminPin() also makes a copy of the Plain Password Memory.
2.) They have to be stored hashed - and not in one place, even better fragmented - inside of the memory.

Axel

[szszszsz]

This is fixed in v1.0. Passwords are handled now more securely and it should be now harder to find them in the released memory.