35

Just saw this in the code I'm reviewing:

function encryptOTP(otp){
var enc = MD5(otp);
return enc;
}

Comments
  • 13
    That's quite a WTF!

    Btw., no hashtags on devRant, this isn't Shitter.
  • 20
    Missing space between ) and {, outrageous
  • 2
    Yikes.
    Are you peer reviewing code for a co-worker or a classmate?
  • 2
    Which part of the code are y’all pissed at?
  • 8
    @hatemyjob The function name suggests to encrypt something when all it does is computing a hash, and even a cryptographically insecure one.
  • 2
    Maybe something from legacy code where it was supposed "to be done later"

    In my app I have something like this for like 2 years :

    function encryptString(str){
    //logic will be added later
    return str;
    }
  • 4
    @NoToJavaScript I would not "brag" about this
  • 1
    @Fast-Nop 'Shitter', ahahahah that had me laughing! :D
  • 1
    @Fast-Nop to be fair, in the security world we use MD5 because it's so good for ID'ing a lot of things... SSL traffic, threat intel, malware, files, etc...

    Really useful hash because of it's ubiquity and it's computational cheapness.

    Not very useful for ENCRYPTION though!
  • 2
    @arcsector Yeah, I even use CRCs for hashing if the amount of items is sufficiently small as not to run into the birthday paradoxon. And both CRCs and MD5 for error protection when copying.
  • 1
    @Fast-Nop i've been having conversations about the lack of built-in checksum support for a lot of modern apps recently, and the answer has always been "nobody uses it"

    To which i respond "How many times have you had to answer this question?" To which the response is either "idk" or "huh, good point". It's a vital part of dynamic apps, and even if your app is built on hashes you should still understand that support makes you a more viable product.
  • 0
    @HelsinkiCodes Security Reviewing
  • 2
    @Jilano Not really. At least the structure is in place. Now when (or if) we actually decide to implement it, all of the code uses it already.
  • 0
  • 2
    Very nifty. The uselessness of the function is well hidden via the introduction of a pointless variable and thus an additiinal line, rather than just doing

    return MD5(otp);

    Brilliant.
Add Comment