ipmitool user set password N: password confirmation doesn't really confirm anything

[AlexanderAmelkin]

Reported by: Vasilii Stepanov Original Ticket: ipmitool/bugs/486

Hi there!

When changing the password for a user you can enter two completely different passwords when prompted for password and confirmation. If you do so, the second will be accepted and be used as the new password. This can be disturbing if someone is doing a typo when specifying the new password.

Reproduction steps:

1. Try to change the password for a selected user, for example: ipmitool -U admin -P admin -H xxx.xxx.xxx.xxx user set password 6
2. Give a password when prompted: Password for user 6: apple
3. Give a different password when prompted for the second time: Password for user 6: grape
4. Try to log in with the new credentials. The password accepted will be "grape".

It seems to be a minor coding/design issue:

int
ipmi_user_password(struct ipmi_intf *intf, int argc, char **argv)
{
        char *password = NULL;
        int ccode = 0;
        uint8_t password_type = 16;
        uint8_t user_id = 0;
        if (is_ipmi_user_id(argv[2], &user_id)) {
                return (-1);
        }

        if (argc == 3) {
        lprintf(LOG_NOTICE, "_0_Entering branch...");
                /* We need to prompt for a password */
                char *tmp;
                password = ask_password(user_id);
                if (password == NULL) {
                        lprintf(LOG_ERR, "ipmitool: malloc failure");
                        return (-1);
                }
                lprintf(LOG_NOTICE, "_1_ password content: %s", password);
                lprintf(LOG_NOTICE, "_2_password lenght: %d", strlen(password));
                tmp = ask_password(user_id);
                if (tmp == NULL) {//TODO
                        lprintf(LOG_ERR, "ipmitool: malloc failure");
                        return (-1);
                }
                lprintf(LOG_NOTICE, "_4_tmp content: %s", tmp);
                lprintf(LOG_NOTICE, "_3_tmp length: %d", strlen(tmp));
                lprintf(LOG_ERROR,  "_5_ password content: %s", password);
                lprintf(LOG_NOTICE, "&password: %x - &tmp: %x", password, tmp);
                if (strlen(password) != strlen(tmp)
                                || strncmp(password, tmp, strlen(tmp))) {
                        lprintf(LOG_ERR, "Passwords do not match.");
                        /* Never-ever gonna happen */
                        return (-1);
                }

In ipmi_user_password function we're using two char* pointers to store the adress of password and the confirmation string (password / tmp). The addresses are returned by ipmi_user_build_password_prompt.

const char *
ipmi_user_build_password_prompt(uint8_t user_id)
{
        static char prompt[128];
        memset(prompt, 0, 128);
        snprintf(prompt, 128, "Password for user %d: ", user_id);
        return prompt;
}

The issue is that in both cases (for password and confirmation) the very same static array's address is being returned. After the password is given ipmi_user_build_password_prompt zeros out the array (losing the password just given) stores the confirmation string and returns it's address.

By the time the control flow reaches the if statement to check the validity, both password and tmp points to the same area in memory:

ipmitool> user set password 3 _0_Entering branch... Password for user 3: 1 password content: apple _2_password lenght: 5 Password for user 3: _4_tmp content: grape _3_tmp length: 5 5 password content: grape &password: 1c80de0 - &tmp: 1c80de0 Set User Password command successful (user 3)

Have a nice day! vassté ( AirFrame Data Center Solutions - NOKIA )