-1

I am trying to implement a straightforward code which return the respective MD5 for a given password:

#include <stdio.h>

//Returns the MD5 hash for a given password
char hash_password(char password){

    FILE *fp;
    char command [1024];
    sprintf(command, "md5 <<< '%c'", password);

    fp = popen(command, "r");
    if (fp == NULL) {
        printf("Failed to run command\n" );
    }

    char hashed[1024];
    fgets(hashed , 1024 , fp);
    pclose(fp);
    return hashed;
}

int main(int argc, const char * argv[]) {
    char hashed = hash_password("password");
    printf("%s\n", hashed);
    return 0;
}

My issues are the following:

  1. I get a warning at return hashed; saying: "Incompatible pointer to integer conversion returning char[1024] from a function with result type char"
  2. I get a warning at char hashed = hash_password("password"); saying: "Incompatible pointer to integer passing char[9] to a parameter of type char"
  3. The program returns \320 which is not the correct hash result.

My two days of experience with C says the function will never return what I need since hashed will die with the end of the function, right? How can I get it fixed?

Malo Polsky
  • 47
  • 9
  • 1
    1. Do not return variables that are on the stack for a function. 2. Get the data types right - the compiler should pick this up – Ed Heal Sep 27 '15 at 13:49
  • Note: MD5 is deprecated as unsafe. – too honest for this site Sep 27 '15 at 13:50
  • 1
    You should check this out: http://www.programiz.com/c-programming/c-pointers-arrays – ergysdo Sep 27 '15 at 13:51
  • 3
    `char` means a single char. `char x[1024];` means an array of 1024 chars. – M.M Sep 27 '15 at 13:51
  • @Olaf - What do you suggest? – Malo Polsky Sep 27 '15 at 14:15
  • 1
    That was just a note you should remember and tell your teacher. If this is just some homework, leave it at that. Otherwise you should take it serious. bcrypt is still deemed safe. Or you use a sha2 hash (but there are also weaknesses, but the longer ones should be safe. However, you should do some research on your own if interested; this cannot be explained in few words. – too honest for this site Sep 27 '15 at 14:24
  • @Olaf Where is the source of your statement? – Malo Polsky Sep 27 '15 at 14:28
  • @marko: Here is **one** http://stackoverflow.com/questions/6774345/md5-security-is-fine/6774381#6774381 – too honest for this site Sep 27 '15 at 14:33
  • It is a best practice to quote a reliable source. – Malo Polsky Sep 27 '15 at 14:36
  • `char hash_password(char password)` What? A single char password? It'll be guessed in nanoseconds. And then you passed into it a full string `hash_password("password");` – phuclv Sep 27 '15 at 14:52
  • 1
    If you're using MD5 for security in any form, switch immediately to something more secure such as PBKDF2 or bcrypt. As for sources, you can find one [here](https://www.kb.cert.org/vuls/id/836068) and another [here](https://tools.ietf.org/html/rfc6151#section-2). The first link contains several links to references itself, including advisories from Microsoft and VeriSign. –  Sep 27 '15 at 14:58

1 Answers1

1
  1. You cannot return a temporary array like that. Instead, have the caller pass in a pointer to an array and then write into it.
  2. Similar to the return type, the parameter cannot be char if you intend to take a string. It should be char* or const char*.
John Zwinck
  • 239,568
  • 38
  • 324
  • 436