Fido icon indicating copy to clipboard operation
Fido copied to clipboard

Hard-Coded Salt Which is Now Public Knowledge

Open sarciszewski opened this issue 9 years ago • 5 comments

I took a quick glance at your AES_Crypto class and discovered the following:

https://github.com/Netflix/Fido/blob/2001671e65c14b6a02b3f41646e07e1d46454a56/Fido_Support/Crypto/AES_Crypto.cs#L28

sarciszewski avatar May 05 '15 19:05 sarciszewski

Reminds me of Uber. Back on topic, what's funny is this is the same code that's on stackoverflow > https://stackoverflow.com/questions/202011/encrypt-and-decrypt-a-string/10366194 (except FIDO's seems to be updated a bit more) scroll down to the first answer and compare the 2.

Also notice the salt is the same, so they could have changed the salt in their production code. It may be safer to change the salt, but considering the encryption I can't say if it honestly matters. AES shouldn't be that easily breakable just because you have the salt. I would say someone with a lot more experience would need to answer that for better information. I just wanted to reply mainly because i just saw this on StackOverflow last friday (question was asked 6 years ago w/ last edit in Oct 2013).

dakre18 avatar May 05 '15 21:05 dakre18

@sarciszewski Thanks for the heads up, this missed my checklist. In a different branch I was using a simple algorithm to create a salt based on local attributes. I'll get that merged back in.

@dakre18, you are correct. The attribution inside this file is missing. Because we don't have a UI I ended up removing much of the encrypt/decrypt functionality, so this class currently is sparsely used. But the intention in the future is to encrypt/decrypt with this class, or something similar, when it comes to sensitive values.

Both points are valid and will be updated, thank you for your feedback.

robfry avatar May 05 '15 21:05 robfry

why did you copy code from stackoverflow into your "security product" in the first place moreover, this makes me wonder where else you have copied code that is used in some of your products from

ghost avatar May 15 '15 23:05 ghost

Kind of a late response, but @blackwat3r I would check what his response was to me. That would show he was planning on using it at some point, but currently is not. Normally when you copy and paste code from the internet, you change it to meet your needs (if needed).

The best way i can put it is why reinvite the wheel? It's fairly common for me to hear that about a lot of things, but this works very well in programming. Why recreate a class that someone else has made and works great?

Also you need to remember when it comes to security, it's best to get working code that's been tested, since you don't want to open up holes in your security because you don't know what you are doing or haven't tested it thoroughly enough.

I hope that clarifies it a bit, even if this response it a bit late.

dakre18 avatar Jul 07 '15 15:07 dakre18

This appears unfixed, six months later.

sarciszewski avatar Nov 16 '15 18:11 sarciszewski