search

Admidio / admidio

Admidio is a free open source user management system for websites of organizations and groups. The system has a flexible role model so that it’s possible to reflect the structure and permissions of your organization.
https://www.admidio.org
GNU General Public License v2.0
307 stars 123 forks source link

Error in creating random int for session id #493

Closed Fasse closed 7 years ago

Fasse commented 7 years ago

view forum https://www.admidio.org/forum/viewtopic.php?f=3&p=25474#p25463

ximex commented 7 years ago

now we have a security issue in v3.2.3. We need infos why this happen.

Fasse commented 7 years ago

In version 3.1 and before we don't have created secure random int. With version 3.2 there are problems at some installations, so in a quick fix we have a fallback, so everyone who uses 3.1 can use 3.2. Now there is time to search for a better fix.

Security is important, but security should not lead to a not usable script.

ximex commented 7 years ago

I need input of the users that have problems with this. I don't want to leave this security leak open forever

ximex commented 7 years ago

@Fasse i want to revert this changes if the users couldn't give an answer they have problems with that... Looks only one user had this problems. It's irresponsible to let this as it is

Fasse commented 7 years ago

If we dont use a PHP Standard than we Need a Fallback!

PeterTheOne commented 7 years ago

random_int is new in PHP7 http://php.net/manual/en/function.random-int.php

ximex commented 7 years ago

and we use a polyfill for PHP 5.x support. So it should work. And if a user couldn't help to find out where the problem is, i couldn't understand why we should open an security hole only for him... This is like: a person often lose his car keys. Ok lets build cars without door locks so this person could drive with our cars. This makes no sense

Fasse commented 7 years ago

@Petertheone I know, but Our minimum PHP Version is Not PHP 7, so if People have Problems we Need a fallback and Not an error.

ximex commented 7 years ago

I will add a big warning message in the admin settings if it isn't possible to generate secure random numbers. With the addition that we won't support this insecure fallback in Admidio 4.0

Fasse commented 7 years ago

@ximex didn't we say that you only implement a warning in 3.2 within the preferences and not set the exception!?!

ximex commented 7 years ago

@Fasse the Exception gets only thrown if you set the new parameter to true. So the default is no Exception. There is no other good possibility to check if prng is secure or not

Fasse commented 7 years ago

Ah sorry, I didn't notice that you call the randomInt within preferences with true. I thought it was while login.