TYPO3 Core

Short note at a first glance:
The encryption key is generated randomly enough when using the install tool (/dev/urandom). @see t3lib_div::generateRandomBytes()

If I correctly understand your report, your overall critic is the usage of md5 for parameter validation in showpic.php?

For further similiar reports, please send an email to the TYPO3 Security Team!

Marcus Krause
Member TYPO3 Security Team

After looking into it again, I cannot find the original indication of a weak encryption key in the current source.

Nevertheless, I don't think there is any reason to ever output client input to an XHTML document without escaping HTML characters. As the comment in showpic.php indicates, the parameter validation is a DoS protection, not an XSS prevention technique.
Additionally, security should not rely on a relatively brittle (compared to no HTML in URLs at all) mechanism which has been shown to have been insecure (http://typo3.org/teams/security/security-bulletins/typo3-sa-2009-001/ ).
On POSIX-incompatible systems, the homebuilt encryptionKey generation scheme results in about 65000 * 1000 = 65 * 10^6 possible key values if the second of installation is known, for example by inadvertent disclosure through wrongly configured file systems or web servers. If the attacker is logged in on a shared system, she might be able to further reduce the number of combinations by watching the pid of the installer or inferring probable pids by other means.

I hope I don't have to argue that md5 is broken as well, see http://en.wikipedia.org/wiki/Md5 for a list of all its flaws.

But that's not the main point; the whole idea of HTML code in URLs is flawed, and bound to result in usability problems, for example when only a part of the URL is copied. The md5 checksum is another problem because it leads to invalid URLs (with a "soft" 200 OK error, therefore not detectable by automatic checkers) when the administrator changes the encryption key.

1. I understand that you dislike the possibility to insert arbitrary html code in the showpic popup. I have to admit that I'm of the same opinion. However, this "feature" is exposed by an API and therefore simply cannot be changed.
Only TYPO3 extension developers or TYPO3 installation admins are able/allowed to configure the html code that is going to be displayed.

2. The mentioned md5 flaws are collision attacks ,where you try to find a modified version of an object that results in the same md5 hash. But in those cases, you have full control of the appearance of the "object".
In TYPO3, you are not aiming to find collisions because you could even use another md5 hash. However, there's always one (constant) part of the object (the encryption key) which the attacker is unaware of and unable to modify.

Then this issue boils down to the question, how secure/insecure/random the encryption key is. And we currently are of the opinion that it is randomly enough.

I/we are really interested in a discussion. Believe us that we treat this issue serious. But for now, we consider this issue as not necessary to be fixed.

PS: The "200 OK" error for a wrong md5 hash is a separate issue and could be addressed in a separate issue.

1. Well, than please make sure this misfeature will not be included in typo3 v5. I updated from 3.8 to 4.2 only to find the old bug of showpic.php moved to a new class. I'm not worried about a practical attack (and all you get is XSS anyway), but also about the clumsiness and possibility of breaking old URLs.

2. Although there may be a number of images on a large site, you are correct in asserting md5 is currently practically safe. Nevertheless, it has been deprecated because of these vulnerabilities which indicate preimage attacks may be achievable in the timespan of typo3v4 installations.

Does anyone use this API for anything but an empty body? Anyway, as long as the next big version of typo3 doesn't contain this misfeature, I'm not interested in any further resolution of this problem. Should I file a second bug for the usability/URL space issues of this construct?

Dear Philipp,

the code was moved to a new class because it seems the author was not aware of this issue. Why should that be clumsy?
Additionally, I don't see where URLs have been broken. Are you referring to the popup links? In this case, I think the problem is really minor (did you ever bookmark a popup image?) Instead, I would always bookmark the parent page, which was never changed due to this, of course.

About having HTML code as part of the URL: It is a fact that many sites are using this feature, e.g. to send a click-on-close event as part of the HTML code. Having HTML in the URL is not very nice and I dislike it myself either, however for compatibility and regarding the low danger of it, I would prefer to keep that option.
However, I propose to introduce pre-defined handlers which could be chosen using a simple parameter and would render the HTML inside the URL parameter obsolete. Examples:
&mode=1 => display body tags, no additional javascript events
&mode=2 => append close-on-click event to the body tag
etc.

3. The "200 OK" error should probably not be fixed, since this will only assist brute-force attackers but is of no help for anyone else.

I wasn't referring to moving the class, but to the solution. Ideally, an URL should look like http://example.com/showpic/lolcat42?width=666&height=auto . The MD5 checksum means the whole URL is necessary. There are people who don't know of c&p, or request only the first line of a URL spread across multiple lines. The checksum increases URL length without benefiting the user.

Some pages may use HTML code in a URL, but I can't imagine any user liking it. They'll probably think "Damn, yet another of those technical overly long words". I like your mode proposal. However, why not default to something sensible, like <body style="background:white; padding:0; margin:0;" js:click-on-close>? That way, we can shave off another parameter whose primary effect is to distract users.

This approach also breaks URLs. People bookmark funny or interesting pictures, and search engines, archives and caches work better if URLs don't change. Encryption keys and therefore showpic URLs should be changed from time to time, not only as a defense-in-depth security measure in the case someone figured out the key, but also in the event of a(nother) vulnerability in the key generation method.

So, do you want to make a patch for this then?
Besides, I think there's no need to discuss the rest any further. I leave the decision open to the security team whether this bug should be unveiled now.
In any case, implementing the mode switch is a new feature and therefore should go into a new bug entry.

- michael

Currently, there are no successful preimage attacks known for md5 hashing. Even if, the way TYPO3 uses md5 hashes for showpic does not qualify for classical preimage attacks.

-> view status set to public

I am closing this issue now. A patch for the usability improvement should be a new issue.

Quoting what the security team says:

"I/we are really interested in a discussion. Believe us that we treat this issue serious. But for now, we consider this issue as not necessary to be fixed."