Geekly Articles each Day
⬆️ ⬇️

Security is not easy

Hello


I present 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 has already been fixed and does not threaten security, but I want to draw the attention of those who do not so often follow the update plug-ins and perhaps do not know about this vulnerability.





Last update: A year later (note of the translator: 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 like generating random numbers and identifiers to ready-made libraries (or at least develop a better way). Many projects have mastered it with difficulty.

')

Let's talk about it, using the example of a function that I met about 6 months ago:

function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }
  1. function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }
  2. function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }
  3. function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }
  4. function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }
  5. function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }
  6. function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }
  7. function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }
  8. function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }
  10. function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }
  11. function generateRandomKey( $len = 20 ) { $string = '' ; $pool = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789' ; for ( $i = 1 ; $i < = $len ; $i ++) { $string .= substr( $pool , rand( 0 , 61 ), 1 ); } return md5( $string ); }


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
Use 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 () several times. This means a big 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 strongly limited range to select random numbers. Also, why choose alphanumeric characters if you don’t use them after they are in the hash function? It seems that this is very badly thought out.

Error 6
Use as a string for md5 alphanumeric characters present in all existing rainbow tables .

Error 7
Never heard of chr () ?

Error 8
Why reinvent the wheel? As a last resort, if you have a reason for this, write about it (this function needs no comments).



Even without checking the results, any programmer should know that this function should 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 PHP documentation, and not creating random number generators, but using one.



Using base_convert () is not really important, mainly because the database field was too short (nothing is lost from sha1 () ). In fact, 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 with 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.



The function is in the sfGuardPlugin and sfDoctrineGuardPlugin for the symfony framework. This function is almost from the beginning , although it was worse . Such today are the most frequently used plugins approved by official documentation.



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



Well, if this is real, someone should have noticed it! I am probably not an intruder.

In fact, it is. I had the opportunity to reproduce all this on the old Debian server, and on this server there was a site where users complained many times that they are logging in as another user (I hope this server has failed and the project is currently working on a fixed function). I also tried to play it on a virtual machine under Windows XP (since it was the easiest way to get PHP with the disadvantages of a random number generator), with success, of course. Since I got a lot of collisions on these two machines, then 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 it.

I think that symfony is great, but security leaves much to be desired (although my other comments concern more standard programming or hosting practices, inspired by symfony and not code).



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

Source: https://habr.com/ru/post/100875/

More articles:



All Articles