Re: [RFC][DISCUSSION] Improve uniqid() uniqueness

Hi, > Actually not with current implementation. If time is rewined, the same ID > could be generated by chance. Because, both ID part and entropy part > is generated based on the current system time. Collision would be rare, > but it's not rare as it should be at all. > > I think minimum would be 64 bits, 128 at least, 256 bits recommended. > > How about use 2nd parameter for entropy length? 0 for none, 1 for > default, 10 to 255 chars. > > Then user can have choice for prefered number of entropy chars. I think, uniqid should be left as is. >>> How serious BC is? >> >> You should already know that this BC-break breaks existing >> valid PHP codes in some situation. (DB error, test failure, etc.) >> >> BC-break may be acceptable if the change is clearly great improvement >> or obviously necessary. But this change is not, I think. > > I do think this is needed. > > Let's not please security audit companies. Use of current uniqid() in > security sensitive context is fatal because it is too easy to predict > generated ID even with "more_entropy". Letting make such mistake > moderate is worth the change. Misuse is just a misuse. The function uniqid() will not be a secure function to be able to use in security sensitive context even if this RFC is passed, isn't it? > In short, making PHP be more secure platform (tolerant even for > mistakes) matter to me. IMO, THIS change is not important enough and not effective enough to do in BC break way. > This BC is nothing compared to mt_rand() everywhere. It would be worth enough. (cryptographically secure random > predictive by seed) > Anyway, let's talk BC with real code. > > I didn't look into all, but only briefly. > > https://searchcode.com/?q=uniqid&loc=0&loc2=10000&lan=24 Why didn't you... You said "BC (BC break) will be minimum"... > I could only find one code that tests uniqid() return value to test uniqid() (?) Example: https://github.com/BrianPrz/worklist/blob/master/classes/Login.class.php In this class, output of uniqid() is saved to DB (maybe). If 'token' field has only 13 length, output length change cause problem. > Other than that, almost all code does not care about uniqid() return > value at all. > Who cares about uniqid() return value? for what purpose? other than testing > uniqid() itself? The uniqid() manual explicitly say, - default value of more_entropy is false - the returned string will be 13 characters long. If more_entropy is TRUE, it will be 23 characters. - if more_entropy is set to TRUE, uniqid() will add additional entropy (using the combined linear congruential generator) at the end of the return value http://php.net/manual/en/function.uniqid.php So, it is fairly valid to design - field length limit to 13 chars in validation code or DB column. - acceptable character type limit to "[0-9A-Za-z]+". - expect increasing value in their PHP applications. -- Kazuo Oishi