4 ms·

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

by amluto 4y ago

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-review-and-why-you-should-avoid-it/ https://paul.reviews/cyberalarm-an-independent-security-revi...

tyingq 4y ago
>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.
- Nextgrid 4y ago
  "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.
  - ev1 4y ago
    If you're going to do the latter at the very least use the built in, included, free libsodium
tgsovlerkhgsel 4y ago
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.
- buu700 4y ago
  They must have been following xkcd's advice: https://xkcd.com/221 https://xkcd.com/221

by amluto 4y ago

tyingq 4y ago

>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.

Nextgrid 4y ago
"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.
- ev1 4y ago
  If you're going to do the latter at the very least use the built in, included, free libsodium

tgsovlerkhgsel 4y ago

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.

buu700 4y ago
They must have been following xkcd's advice: https://xkcd.com/221 https://xkcd.com/221