0

Trying to learn something here more than solve the specific problem. Please help me towards some best practices to apply in this situation, and if possible some clarification of why. Thanks in advance for your assistance.

Basically I'm brute-force breaking a very simple hash algorithm, within known limits. Function tests possibilities of a string (within a length limit) against the hash, until it matches the hash passed. Then recursion should stop all iterations and return the string that matched. The iteration works, but when the answer is found, it seems that each run of the function doesn't get the value returned by its call of the same function.

Here's the code of the function, with extra comments for clarity:

//'hash' is the hash to be replicated
//'leading' is for recursive iteration (1st call should have leading=="")
//'limit' is the maximum string length to be tested

string crack(string hash, string leading, int limit)
{
    string cracked=NULL, force=NULL, test=NULL;

    //as per definition of C's crypt function - validated
    char salt[3] = {hash[0], hash[1], '\0'};

    // iterate letters of the alphabet - validated
    for(char c='A'; c<='z'; c++)
    {
        // append c at the end of string 'leading' - validated
        test = append(leading,c);

        // apply hash function to tested string - validated
        force = crypt(test,salt);

        // if hash replicated, store answer in 'cracked' - validated
        if(strcmp(hash,force)==0)
        {
            cracked = test;
        }
        else
        {
            // if within length limit, iterate next character - validated
            if(strlen(test)<=limit+1)
            {
                // THIS IS WHERE THE PROBLEM OCCURS
                // value received when solution found
                // is always empty string ("", not NULL)
                // tried replacing this with strcpy, same result
                cracked = crack_des(hash,test,limit);
            }
        }

        // if answer found, break out of loop - validated
        if(cracked){break;}

        // test only alphabetic characters - validated
        if(c=='Z'){c='a' - 1;}
    }

    free(test);

    // return NULL if not cracked to continue iteration on level below
    // this has something to do with the problem
    return cracked;
} // end of function

From the little bit I recall of pointers, I'd guess it is something with passing references instead of values, but I don't have enough knowledge to solve it. I have read this thread, but this suggestion doesn't seem to solve the issue - I tried using strcpy and got the same results.

Disclaimer: this is an exercise in 2018's CS50 by Harvard at EDX. It won't affect my grading (have already submitted two perfect exercises for the week, which is what is required) but as stated above I'm looking to learn.

Edit: edited the tag back to C (as clarified in comments, string is from string.h, and append was coded by me and validated several times over - I'll get to TDD in a while). Thanks all for your comments; problem solved and lessons learned!

11
  • 6
    Did someone add a string type to C when I wasn't looking? Or is that a typedef for char*? Commented Aug 27, 2018 at 3:13
  • 2
    You mean C++? There is no string in C. Commented Aug 27, 2018 at 3:13
  • ...and so far as I know, there's no operator for conversion in a bool context for the one in C++, so if(cracked){break;} is suspect if the intent was C++. Everything in the code seems consistent with it being a typdef. Commented Aug 27, 2018 at 3:21
  • Changed the c tag to c++. The append() method is not a C method. It is a C++ method. Commented Aug 27, 2018 at 3:26
  • 1
    Fix the submission so that there is a recursive function in it. It would also help to know how string and append are defined. Commented Aug 27, 2018 at 3:33

1 Answer 1

4

I found a bug in the code, but I am not sure whether it is the root cause of your problem.

When the code hit the line:

strcmp(hash,force)==0

then you will assign the string pointed by 'test' to 'cracked':

cracked = test;

then this line is hit:

if(cracked){break;}

then the loop is breaked, and the next line:

free(test);

this line will free the string pointed by test, and remember that it is the same string pointed by 'cracked', thus you returned a string which is already freed.

What will happened to the string is dependent on your compiler and libc. You can try to fix this problem by allocating memory for 'cracked':

cracked = strdup(test);

Also, there are memory leaks caused by the 'test' and 'force' string, but they should be irrelevant to your problem.

Sign up to request clarification or add additional context in comments.

7 Comments

Did you assume string is std::string here? . If then cracked = test; does a copy, not keeping pointer. But I don't understand why free(test) is there for strings.
@PraAnj He is operating the strings with strcmp(), free the string by free(), and the tag is "C" instead of "C++", so I regard string as an typedef of char *
@PraAnj When I was answering the question, the tag is still "C", now it is edited by other people. I think it is quite obvious that in the question, the author is using strcmp/strlen/free to operate the "string" type. typedef char * is the only reasonable guess. And editing the tag to C++ without further confirm with the author is not responsible.
@YiZhenfei the OP has made no attempt to deny nor confirm whether their source code was C. Given the information that we have, it's safe to assume that this is a C++ post. I agree with your statement; the tags will be edited again, if need be, when the OP states more information.
Thanks @YiZhenfei, indeed the freeing of test was the issue. I had had a full execution fail before and imagined it might be due to the memory leak caused by test and force, as you mentioned.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.