One more auth system that will give you nightmares...

This JavaScript code powers a 1,500 user intranet application.

The longer you look at it the more insane it gets.

@LoganDice The icing on the cake is the todo comment on top.

Like putting it in a different file is the LEAST of your concerns here. 🤣

@logicallysound @LoganDice even more egregious is the use of an HTML comment... you could just use a JS comment inside the script tag

@jacobherrington @LoganDice Honestly I think that's less egregious. The HTML comment is more visible on a quick scan of the overall HTML file than a JS comment inside the script would be and it's easier to interpret the TODO as applying to the script tag as a whole.

@logicallysound lol I just think <!-- --> is a terrible way to write comments. 😆 Code style is 99% preference.

@jacobherrington I've never liked how HTML comments look so I can sympathize on that point 😅

@LoganDice It went non-Euclidean as soon as I opened the image. That code is whispering in the back of my mind, demanding that I yield myself to Yog-Sothoth.

@starbreaker @LoganDice

I saw this image circulating earlier, but didn't realize it made it into production. Wow.

@rick_777 @LoganDice This is why I carry a M-79 grenade launcher in my backpack along with my laptop. :)

@LoganDice Why are there 3 ='s for the logic tests?

(yes, there is a LOT more here....)

@kemonine @LoganDice actually, `===` is usually a good idea because you avoid accidental type conversions.

@kemonine @LoganDice in JS, double equals also matches coerced values so 0 == “0” will return true

@kemonine In JavaScript, `==` performs coercions so that e.g. `2 == "2"` is true. The `===` doesn't, acting much more like `==` in other C-derived languages.

@LoganDice my favorite part is the three exclamation point todo at the top

@csalzman @LoganDice Yeah, definitely put this in a different file, and then delete that file.

@LoganDice if ("true" === "true") { return false; }

*chef kiss*

@LoganDice wtf it just iterates over every account until it hits the right one??? with 1500 users???? also what the hell is going on at line 556? shouldnt there be hashing involved?



Not to mention that it's a frontend code, so anyone can see all the user's info. And execute arbitrary sql, if you want to drop all tables for example.

@LoganDice if I understand whats happening here then.... Fired 😵

@LoganDice There's like 70+ levels of WTF in this compressed to just those few lines.

@LoganDice Psh, just put it behind https and it's 100% secure not crackable perfect security.

That's how that works right? Anyone? No?

@LoganDice I am not sure why but I kind of want to frame this and hang it on my wall 😐 Can't tell what's wrong with me 🙄

All vars are declared. Comparisons are made with ===. Code is formatted nicely. It looks like it should work. What’s the problem? There’s no problem here! 

@LoganDice </sarcasm>

@LoganDice Are... are you going to fix it? Please?

@LoganDice i was gonna mention that at least it doesn't seem that there's an injection vulnerability, then i fully caught on that this is running in the client...

@brennen @LoganDice and a db where the passwords are stored in plaintext, goddess almighty 😰

@brennen @LoganDice why yes i will happily slurp every password on login, thank you goodbye

@LoganDice A reply from a friend of mine who read this toot:

"Don't think of it as a security vulnerability, think of it as backing up your user database on every client" 🤣

@LoganDice I think at some point in my life I have written intranet code like this. (Not in JavaScript though. JavaScript didn't exist back then.)

@LoganDice it will be way more secure when moved to another file!

@LoganDice raw sql exposed, severe language weirdness, unhashed passwords of all users sent to anyone on every login attempt and then "logged in" is just a cookie you can set arbitrarily anyways and there's *still* more…

Truly astounding – it's like a masterpiece of horrible software engineering.

@LoganDice In JavaScript, “true === true” equates to NaN.

(This is a joke)

(... or is it ...?)

@LoganDice that's the good shit right there. SQL injection as a service!

@LoganDice is there an actual purpose to `if ("true" === "true") {`, or is it just more bonkers shit?


Hmmm. Okay, they're not hashing the password, which is bad. And the code has a bunch of pointless stuff, but I don't see---AAAAAAH IT'S RUNNING ON THE CLIENT!!!!!!


I'm kinda curious about that "True" === "True" at the bottom. Is that an established JavaScript idiom with a well known purpose? Or is it just there?

@mds2 @LoganDice technically it evaluates to true, but it's not remotely ideal.

@LoganDice please tell us where this code is deployed??

@LoganDice this is wrong on so many levels!
- passwords are not encrypted.
- open to sql injection (there still can be mean people on intranet)
- you are fetching the entire datatable!

My eyes are bleeding at the moment. And that’s not because of the monokai theme :)

Sign in to participate in the conversation

Server run by the main developers of the project 🐘 It is not focused on any particular niche interest - everyone is welcome as long as you follow our code of conduct!