Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
suresh-kumara-gist opened this issue Sep 18, 2018 · 6 comments
Closed

Comments

@suresh-kumara-gist
Copy link

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
Copy link
Member

cweiske commented Sep 18, 2018

Where exactly is the security problem here?

@tuxun
Copy link

tuxun commented Sep 18, 2018

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
Copy link
Member

cweiske commented Sep 18, 2018

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

@suresh-kumara-gist
Copy link
Author

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
Copy link
Member

cweiske commented Sep 19, 2018

  • you provided evidence that a not-so-secure ID attribute is a security risk
  • you understand the problem

@cweiske cweiske closed this as completed Sep 19, 2018
@BenBE
Copy link
Contributor

BenBE commented Oct 1, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants