Comment by amluto | Hacker News Reader

Comment by amluto | Hacker News Reader

I admit that, reading the code, I found it challenging to find a severe crypto bug simply because everything is wrong. Even their way of packing the data into a base64 is nonsense. The parser is nonsense. Almost every line of code is wrong. Without more context, I’m not quite convinced that the key is reliably anything other than a constant due to the use of PHP and the failure to validate that the variables in question are initialized.

It’s like the most severe bugs are buried in code that is 100% bug!

edit: the key is a constant. I’m speechless. See https://paul.reviews/cyberalarm-an-independent-security-revi...

No, no, the key is not a constant. See, they already changed it once! /s

(Honestly, that was the biggest red flag for me. They had to change the key and that STILL didn't give them the hint that hardcoding it is not a sane option.)

Any company that was involved in this disaster and either implemented or gave their seal of approval to hardcoded crypto keys needs to be permanently excluded from government contracts.

They must have been following xkcd's advice: https://xkcd.com/221

>due to the use of PHP

PHP allows using uninitialized variables, but it does spew notices:

  PHP Notice:  Undefined variable: bar in testme.php on line2

And most implementations of this would probably be using objects, properties, property_exists(), etc.

"Proper" PHP with a modern framework such as Laravel is decent and as far as I know would forbid this (there's a global exception handler that returns a 500 page with a stack trace).

Bare PHP with no framework or anything to patch PHP's shortcomings is IMO unacceptable in today's day and age.

If you're going to do the latter at the very least use the built in, included, free libsodium

Reply on news.ycombinator.com