Project

General

Profile

Actions

Bug #50929

closed

Don't call rawurlencode() twice on curUrl

Added by Jan Spisiak almost 11 years ago. Updated almost 11 years ago.

Status:
Closed
Priority:
Must have
Assignee:
Category:
-
Target version:
-
Start date:
2013-08-08
Due date:
% Done:

0%

Estimated time:
1.00 h
TYPO3 Version:
6.1
PHP Version:
Tags:
Complexity:
no-brainer
Is Regression:
Yes
Sprint Focus:

Description

In ElementBrowser.php:

$this->setTitle = $this->curUrlArray['title'] != '-' ? rawurlencode($this->curUrlArray['title']) : '';
...
$JScode = '
    ...
    var add_title="' . ($this->setTitle ? '&curUrl[title]=' . rawurlencode($this->setTitle) : '') . '";
    ...
';

This is for all curUrl attributes. In the end this means that javascript variables are encoded twice and the prefilled form is also encoded even if it shouldn't be.
I've included patch, but I am not sure if it satisfies all the rules.


Files

patch1.diff (1.75 KB) patch1.diff Jan Spisiak, 2013-08-08 16:27

Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #41413: link title got urlencoded in link-wizardClosed2012-09-27

Actions
Actions #1

Updated by Markus Klein almost 11 years ago

This double encoding was introduced in a security fix in 2011.

http://review.typo3.org/3754

I believe this fix was wrong, since it should protect against XSS. Double encoding of course does the trick, but it would IMHO be better to simply htmlspecialchar the output.

Actions #2

Updated by Jan Spisiak almost 11 years ago

Yes, that is the fix that broke it. Well the output into the form is already htmlspecialchar'ed:

<td><input type="text" name="ltitle" class="typo3-link-input" onchange="browse_links_setTitle(this.value);" value="' . htmlspecialchars($this->setTitle) . '" /></td>

But there is also the cur_* variables:
var cur_title="' . ($this->setTitle ? $this->setTitle : '') . '";

These are not rendered, they are just sent through renderPopup_addLink() back to editor, so should they also be htmlspecialchar'ed ?

Actions #3

Updated by Jan Spisiak almost 11 years ago

  • Assignee set to Georg Ringer
  • Target version changed from 2463 to next-patchlevel

This needs to be fixed, otherwise it renders editing links impossible when using ElementBrowser (Flux uses it).

Actions #4

Updated by Georg Ringer almost 11 years ago

  • Is Regression set to No

please never assign any issues to someone else then yourself!

Actions #5

Updated by Oliver Hader almost 11 years ago

  • Target version deleted (next-patchlevel)
Actions #6

Updated by Georg Ringer almost 11 years ago

  • Is Regression changed from No to Yes
Actions #7

Updated by Oliver Hader almost 11 years ago

  • Status changed from New to Accepted
Actions #8

Updated by Oliver Hader almost 11 years ago

  • Status changed from Accepted to Closed

See issue #41413

Actions

Also available in: Atom PDF