GeSHi / geshi-1.0

Original version of Generic Syntax Highlighter for PHP
http://qbnz.com/highlighter/
GNU General Public License v2.0
168 stars 100 forks source link

Use of a Broken or Risky Cryptographic Algorithm in geshi/geshi.php #109

Closed suresh-kumara-gist closed 5 years ago

suresh-kumara-gist commented 5 years ago

Use of a Broken or Risky Cryptographic Algorithm in geshi/geshi.php. line number 3827.

$this->overall_id = 'geshi-' . substr(md5(microtime()), 0, 4);

cweiske commented 5 years ago

Where exactly is the security problem here?

tuxun commented 5 years ago

maybe substr(0,4) from md5(randomstuff) is too predictible, doesn't this let us with something like 36^4 possibility "only"??? https://phpsecurity.readthedocs.io/en/latest/Insufficient-Entropy-For-Random-Values.html and i found http://php.net/manual/fr/function.microtime.php#101561

While doing some experiments on using microtime()'s output for an entropy generator I found that its microsecond value was always quantified to the nearest hundreds (i.e. the number ends with 00), which affected the randomness of the entropy generator.

cweiske commented 5 years ago

And where exactly is a random HTML id attribute a security problem?

suresh-kumara-gist commented 5 years ago

md5(microtime()) This function uses the !php_standard_ns.md5() function, which uses a hash algorithm that is considered weak. In recent years, researchers have demonstrated ways to breach many uses of previously-thought-safe hash functions such as MD5.

cweiske commented 5 years ago
BenBE commented 5 years ago

The provided algorithm is for generating a set of random IDs in case where the user does not provide a custom one, but where still the code needs to generate some (somewhat) unique ID. As the ID is truncated to be short even using something "more secure" like SHA-256 or Keccak won't solve the problem of collissions. The only solution really fixing the issue would be for the user of the library to provide GeSHi with a guaranteed-random base ID. In absence of a guaranteed-unique value provided by the caller, reasonably trying to generate something random is sufficient (and NOT a security issue); i.e. the worst that can happen is the UA rendering the returned string switching to Quirks Mode due to duplicated generated IDs in the document.