Skip to main content
Commonmark migration
Source Link

###Structure

Structure

So, our smallest result is 32--a little more than the "28" that's left over if we divide 628 by 6, so we want something that produces a result a little less than 100. A quick glance shows that 1 produces a result of 87--that'll probably do. So, if we start with 111111, we should get a result of 6 * 87 = 522. That gives us 106 left over. Another glance through the table shows that "R" gives 106, so an input of 111111R should give us 628. A quick run with std::cout << HashPassword("111111R"); verifies that it does exactly that.

###Structure

So, our smallest result is 32--a little more than the "28" that's left over if we divide 628 by 6, so we want something that produces a result a little less than 100. A quick glance shows that 1 produces a result of 87--that'll probably do. So, if we start with 111111, we should get a result of 6 * 87 = 522. That gives us 106 left over. Another glance through the table shows that "R" gives 106, so an input of 111111R should give us 628. A quick run with std::cout << HashPassword("111111R"); verifies that it does exactly that.

Structure

So, our smallest result is 32--a little more than the "28" that's left over if we divide 628 by 6, so we want something that produces a result a little less than 100. A quick glance shows that 1 produces a result of 87--that'll probably do. So, if we start with 111111, we should get a result of 6 * 87 = 522. That gives us 106 left over. Another glance through the table shows that "R" gives 106, so an input of 111111R should give us 628. A quick run with std::cout << HashPassword("111111R"); verifies that it does exactly that.

Source Link
Jerry Coffin
  • 33.6k
  • 4
  • 75
  • 143

###Structure

Okay, let's start with the basic overall structure of the function: it's, frankly, not very good. It (unnecessarily) depends on a number of outside variables--from its perspective, essentially globals.

There's no good reason that this couldn't be a nice, pure function--i.e., take an input parameter, do the hash, return a result. No reference to anything outside the function at all.

int HashPassword(std::string const &Combine) { 
    unsigned int hash = 0;

    const unsigned int VALUE = Combine.length();
    for (auto Letter : Combine)
    {
        srand(VALUE*Letter);
        hash += 33 + rand() % 92;
    }
    return hash;
}

Naming

Some of the names are pretty good (e.g., Letter, HashPassword) but others (e.g., VALUE, Combine) seem like they're open to some improvement. I can see how you came up with Combine, but that doesn't really make it a good name. VALUE is so generic that it's probably best to avoid it except (perhaps) in something like a class that represents some number, where the VALUE of the object has a fairly obvious meaning.

Magic numbers

The 33 and 92 are a couple of examples of magic numbers. These generally lead to code that's difficult to read (at best).

Security

This hash function provides approximately what's usually referred to as "kid sister" level security. If you have a kid sister, this might be about enough to keep her fooled--but only if she's not very bright or fairly quickly bored.

The most obvious problem is that this just generates a value from each byte, then adds those values together. This means nobody really needs to know your actual password--at most they only need to care about the letters that make up the password. For example, HashPassword("ABC"), and HashPassword("CAB") both produce exactly the same result.

So, to get the right result, we don't really even need to guess their password--we just need to guess some characters that produce the right result--and since nearly every input in the right range maps to a unique result, it doesn't take a lot to come up with a usable result either. For example, let's consider a result of 628. Each input produces a result from 32 to 124, so we might want to use, say, seven characters in a password. So, let's start by computing the result for seven repetitions of each character in the most common range:

for (int i = '0'; i < 'z'; i++) {
    std::string in(7, (char)i);
    std::cout << "in: " << in[0] << ", hash: " << HashPassword(in) / 7 << "\n";
}

So, our smallest result is 32--a little more than the "28" that's left over if we divide 628 by 6, so we want something that produces a result a little less than 100. A quick glance shows that 1 produces a result of 87--that'll probably do. So, if we start with 111111, we should get a result of 6 * 87 = 522. That gives us 106 left over. Another glance through the table shows that "R" gives 106, so an input of 111111R should give us 628. A quick run with std::cout << HashPassword("111111R"); verifies that it does exactly that.

In case it seems like I hand-picked something that would be easy, I'll point out that I actually got the 628 from hashing "Password"--so to find 111111R, I really did start over from scratch--and I didn't even have to work hard at it--my first attempt worked out easily.

For another approach, I could have started with 5 of something. Let's say 'j', which gives 102. 5 x 102 = 510. I want two more characters to make up the remainder, so for my next character I want to pick something fairly small. Let's say _, which produces 35. With the previous 510, we get 545. A quick subtraction shows that we need 83 more, and a quick search shows that "M" produces 83, so jjjjj_M would work as well (and a quick test verifies that yes, it also produces 628).

Bottom line: from a security viewpoint, this is so weak that it's open to question whether it really even qualifies as "kid sister" level, or if it's too weak to even qualify for that level of "security".