Security is not easy.

Original author: Laurent Bachelier
  • Transfer
Hello.

I present to you a translation of a rather interesting article that I found trying to fix a bug in sfDoctrineGuardPlugin. Fortunately, about 2 weeks ago everything written here was already fixed and nothing is in danger, but I want to draw the attention of those who do not often monitor plug-in updates and may not know about this vulnerability.


Last update: One year later (both translator's notes: after the first publication of the error), both plugins are finally updated and use a better random key generator.

Security is not easy. Programmers should leave things such as random number and identifier generation ready-made libraries (or at least develop a better way). Many projects have mastered this with difficulty.

Let's talk about this, using the example of a function that I met about 6 months ago:
  1. function generateRandomKey($len = 20)
  2. {
  3.   $string = '';
  4.   $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789';
  5.   for ($i = 1; $i < = $len; $i++)
  6.   {
  7.     $string .= substr($pool, rand( 0, 61), 1);
  8.   }
  9.  
  10.   return md5($string);
  11. }
  12.  

This is a real gem, as it seems, it was written to focus everything that a programmer should not do.
Error 1
Using rand () instead of mt_rand () . Any PHP programmer should know this.
Error 2
Using md5 () instead of sha1 () . Any PHP programmer should know this.
Error 3
Creating a 20 character random string, and then hashing it into a 32 character string, looks a bit silly. Of course, the hash contains fewer different characters, but still at the input we have 7.04423e +35 different values, and at the output we can have 3.40282e +38.
Error 4
Call rand () multiple times. This means a great disregard for how pseudo random number generators work. Until the source changes, it will be possible to easily guess what the next number will be, depending on the previous ones. It will not be "more random" with multiple function calls.
Error 5
Use a very limited range to select random numbers. Also, why choose alphanumeric characters if you do not use them after they switch to the hash function? It seems like this is very poorly thought out.
Error 6
Using the alphanumeric characters present in all existing rainbow tables as a string for md5 .
Error 7
Ever heard of chr () ?
Error 8
Why reinvent the wheel? In extreme cases, if you have a reason for this, write about it (this function does not need comments).

Even without checking the results, any programmer should know that this function must be completely rewritten.

Let's see how it should have looked.
  1. function generateRandomKey()
  2. {
  3.   return base_convert(sha1(uniqid(mt_rand(), true)), 16, 36);
  4. }

How did I get to this? Basically, reading the documentation for PHP, and not creating random number generators, but using one.

Using base_convert () is actually not important, mainly because the database field was too short (nothing is lost from sha1 () ). Actually, it would be better without sha1 () , we use it only for beauty.

uniqueid () takes the value obtained from mt_rand () and adds a random unique value to it (but in a predictable range, it makes sense to use both functions). This is recommended by the PHP documentation and this is more than enough.

Now, you probably ask yourself where I found this piece of code.

Function is insfGuardPlugin and sfDoctrineGuardPlugin for the symfony framework. This feature here is almost from the beginning , although it was worse . These are by far the most commonly used plugins approved by official documentation.

What does it mean? On some systems, it will be very simple to log in as any user using brute force. Or the user may accidentally log in as another, since the likelihood of a collision is very high.

Well, if it's real, someone should have noticed it! I am probably not an attacker.
In fact, it is. I had the opportunity to reproduce all this on an old Debian server, and on this server there was a site where users complained many times that they were logging in as another user (I hope this server failed and the project is currently working on a fixed function). I also tried to play it in a virtual machine under Windows XP (since it was the easiest way to get PHP with the flaws of a random number generator), with success, of course. Since I got a lot of collisions on these two machines, most likely, even on more advanced systems, the error can be used with some effort.

Why am I doing this?
Because I reported this six months ago, but little attention was paid to this.
I think symfony is great, but security is poor (although my other comments are more about standard programming or hosting practices, inspired by symfony rather than code).


On this topic, there is another post where an example of the work of a miracle function is given.

Also popular now: