HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMajor

Custom algorithm for hashing and un-hashing password

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
andpasswordcustomalgorithmforhashing

Problem

I am attempting to create functions for getting an algorithm, hashing and un-hashing a string.

As it stands, it works, and very well. But, I feel this is not secure enough and someone could easily break it!

Can someone help me to improve this?

Code:

``
"[H]",
"b"=>"[He]",
"c"=>"[Li]",
"d"=>"[Be]",
"e"=>"[B]",
"f"=>"[C]",
"g"=>"[N]",
"h"=>"[O]",
"i"=>"[F]",
"j"=>"[Ne]",
"k"=>"[Na]",
"l"=>"[Mg]",
"m"=>"[Al]",
"n"=>"[Si]",
"o"=>"[P]",
"p"=>"[S]",
"q"=>"[Cl]",
"r"=>"[Ar]",
"s"=>"[K]",
"t"=>"[Ca]",
"u"=>"[Sc]",
"v"=>"[Ti]",
"w"=>"[V]",
"x"=>"[Cr]",
"y"=>"[Mn]",
"z"=>"[Fe]",
"A"=>"[Co]",
"B"=>"[Ni]",
"C"=>"[Cu]",
"D"=>"[Zn]",
"E"=>"[Ga]",
"F"=>"[Ge]",
"G"=>"[As]",
"H"=>"[Se]",
"I"=>"[Br]",
"J"=>"[Kr]",
"K"=>"[Rb]",
"L"=>"[Sr]",
"M"=>"[Y]",
"N"=>"[Zr]",
"O"=>"[Nb]",
"P"=>"[Mo]",
"Q"=>"[Tc]",
"R"=>"[Ru]",
"S"=>"[Rh]",
"T"=>"[Pd]",
"U"=>"[Ag]",
"V"=>"[Cd]",
"W"=>"[In]",
"X"=>"[Sn]",
"Y"=>"[Sb]",
"Z"=>"[Te]",
" "=>"[I]",
"1"=>"[Xe]",
"2"=>"[Cs]",
"3"=>"[Ba]",
"4"=>"[Hf]",
"5"=>"[Ta]",
"6"=>"[W]",
"7"=>"[Re]",
"8"=>"[Os]",
"9"=>"[Ir]",
"0"=>"[Pt]",
"!"=>"[Au]",
"£"=>"[Hg]",
"$"=>"[Tl]",
"%"=>"[Pb]",
"^"=>"[Bi]",
"&"=>"[Po]",
"*"=>"[At]",
"("=>"[Rn]",
")"=>"[Fr]",
"_"=>"[Ra]",
"-"=>"[Rf]",
"="=>"[Db]",
"+"=>"[Sg]",
"
"=>"[Bh]",
"¬"=>"[Hs]",
""=>"[Mt]",
","=>"[Ds]",
""[Rg]",
"."=>"[Cn]",
">"=>"[Uut]",
"/"=>"[Fl]",

Solution

First of all, what you have is an encryption function, not a hashing function (encryption can be reversed by knowing the algorithm and optionally a password, hashing is one way).

For passwords, you always* want to hash, not encrypt (because if an attacker gets your source code, they can easily decrypt the passwords).

You are using a substitution cipher, which is also very easy to break. You might want to look into block or stream ciphers, if you want encryption that is more secure.

As to the code you do have:

  • function names should start with a lower-case character (classes start with an uppercase one).



  • with the information given above, it's obvious that the function names don't really fit. Also, you don't need filler words such as It in function names. Something like encrypt(plaintext) and decrypt(ciphertext) would be better.



  • You shouldn't silently ignore errors. I would add an else clause to your if ($find) and throw an exception.



  • In UnhashIt you check if the string exists, but you don't do it for the more important HashIt function. This means that UnhashIt(HashIt(text)) != text in some cases, which is not good.



  • For any real-world code, use bcrypt.



* there are exceptions such as kerberos and password managers, but in use cases where you want to authenticate a user by checking a user supplied password against a stored password (ie whenever you do have a user supplied plaintext password to check against), hashing is the correct way, not encryption
(see the comments/chat for an extended discussion)

Context

StackExchange Code Review Q#114881, answer score: 25

Revisions (0)

No revisions yet.