Bug #20529

stdWrap for TypoSrcrip-select parameters

Added by David Bruchmann about 12 years ago. Updated over 10 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
-
Target version:
-
Start date:
2009-05-29
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
4.3
PHP Version:
5.2
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

The select function doesn't allow stdWrap for several parameters. So the have to be hardcoded in TS.

I changed function getQuery in class.tslib_content.php so that all parameters in the defined array $stdWrapAllowedValues are parsed through stdWrap.

Now im'm not quite sure, which parameters kann be added there, adding the sql-statements (where, andwhere, join) brakes down the function but what's about begin, languageField, orderBy, groupBy?

function getQuery($table, $conf, $returnQueryArray=FALSE) {

$stdWrapAllowedValues = Array('pidInList.','uidInList.','recursive.','min.','max.');
foreach ($conf as $k => $v) {
if (in_array($k,$stdWrapAllowedValues) || in_array($k.'.',$stdWrapAllowedValues)) {
preg_match('/(.*?)\./',$k,$r);
$conf[$r[1]] = trim($this->stdWrap($conf[$r[1]],$conf[$k]));
}
}
// Handle recursive function for the pidInList
if (isset($conf['recursive'])) {
$conf['recursive'] = intval($conf['recursive']);
if ($conf['recursive'] > 0) {
foreach (explode(',', $conf['pidInList']) as $value) {
$pidList .= $value . ',' . $this->getTreeList($value, $conf['recursive']);
}
$conf['pidInList'] = trim($pidList, ',');
}
}
if (!strcmp($conf['pidInList'],''))    {
$conf['pidInList'] = 'this';
}
$queryParts = $this->getWhere($table,$conf,TRUE);
// Fields:
$queryParts['SELECT'] = $conf['selectFields'] ? $conf['selectFields'] : '*';
// Setting LIMIT:
if ($conf['max'] || $conf['begin']) {
$error=0;
// Finding the total number of records, if used:
if (strstr(strtolower($conf['begin'].$conf['max']),'total')) {
$res = $GLOBALS['TYPO3_DB']->exec_SELECTquery('count(*)', $table, $queryParts['WHERE'], $queryParts['GROUPBY']);
if ($error = $GLOBALS['TYPO3_DB']->sql_error()) {
$GLOBALS['TT']->setTSlogMessage($error);
} else {
$row = $GLOBALS['TYPO3_DB']->sql_fetch_row($res);
$conf['max'] = str_ireplace('total', $row[0], $conf['max']);
$conf['begin'] = str_ireplace('total', $row[0], $conf['begin']);
}
$GLOBALS['TYPO3_DB']->sql_free_result($res);
}
if (!$error) {
$conf['begin'] = t3lib_div::intInRange(ceil($this->calc($conf['begin'])),0);
$conf['max'] = t3lib_div::intInRange(ceil($this->calc($conf['max'])),0);
if ($conf['begin'] && !$conf['max']) {
$conf['max'] = 100000;
}
if ($conf['begin'] && $conf['max'])    {
$queryParts['LIMIT'] = $conf['begin'].','.$conf['max'];
} elseif (!$conf['begin'] && $conf['max']) {
$queryParts['LIMIT'] = $conf['max'];
}
}
}
(issue imported from #M11220)

Files

bug_11220.diff (935 Bytes) bug_11220.diff Administrator Admin, 2009-05-29 16:17
bug_11220_2.diff (7.57 KB) bug_11220_2.diff Administrator Admin, 2009-05-30 16:36
bug_11220_3.diff (7.95 KB) bug_11220_3.diff Administrator Admin, 2009-05-30 19:44
bug_11220_4.diff (7.93 KB) bug_11220_4.diff Administrator Admin, 2009-05-30 20:55
bug_11220_5.diff (7.94 KB) bug_11220_5.diff Administrator Admin, 2009-05-31 19:54
bug_11220_6.diff (8.13 KB) bug_11220_6.diff Administrator Admin, 2009-05-31 21:33
bug_11220_8.diff (8.23 KB) bug_11220_8.diff Administrator Admin, 2009-06-07 20:49

Related issues

Related to TYPO3 Core - Feature #20508: Enable stdWrap on select properties groupBy and orderByClosedOliver Hader2009-05-26

Actions
Is duplicate of TYPO3 Core - Feature #24089: Optimize stdWrap usage for tslib_contentClosedSusanne Moog2010-11-17

Actions
#1

Updated by Oliver Hader about 12 years ago

Concerning stdWrap functionality for groupBy and orderBy there's already a patch pending in the core list as RFC #20508

#2

Updated by David Bruchmann about 12 years ago

Hello Oliver,

Your Solution is a particial one. I don't see any reason to restrict changes to the two parameters you choosed. Another Bug also solved a single Option: http://bugs.typo3.org/view.php?id=7921.
I have another Solution respecting all possible Parameters but this is incompatible with yours because for some parameters I integrated the choice to stdWrap it or not - that's not possible with your proposition.
Furthermore I added two more parameters to influence behavior:
1) $conf['pidInList']['dontSetPid'] to avoid that empty pidInList is filled with "this"
2) $conf['andWhere']['dontUseStdWrap'] to avoid that addWhere is stdWrap_ped by default

I'll posted a new patch (_2), futhermore a small extension to test different queries will coming soon.

#3

Updated by David Bruchmann about 12 years ago

Mhm, something still isn't perfect with patch 2. Damned.

Uploaded patch 3 where some faulting behavior in loop was fixed.

Will test more.

#4

Updated by David Bruchmann about 12 years ago

found one typo and shortend inquiries a bit in patch 4
Tests I build until now are proceeded correct

changed parameter-name to avoid misunderstanding in patch 5

corrected search for parameters in lines 7166, 7167 in patchh 6

#5

Updated by tyler kraft about 12 years ago

+1

I'd like to see this in 4.3

#6

Updated by Jo Hasenau about 12 years ago

As long as TypoScript select is using stdWrap without escaping the results of the stdWrap functions before executing the SQL query, any stdWrap call for any select parameter IMHO is a no go!

select.blah.data = GPvar:blah

will open up a nice way for MySQL injections without the admin ever noticing it, since he might not have access to the source to check, if the values are escaped. Many people might think they are escaped and rely on the core to handle the stuff properly (which it does not) - so there should be a fix to the existing problem before adding even more options to create security holes.

#7

Updated by David Bruchmann about 12 years ago

Hi Jo,

what do you think about "where", "andWhere" and the different joins referring stdWrap?

Regards
David

#8

Updated by David Bruchmann about 12 years ago

It's simple to change some lines like that:

from
// Fields:
$queryParts['SELECT'] = $config['selectFields'] ? $config['selectFields'] : '*';

to this:
// Fields:
$queryParts['SELECT'] = $config['selectFields'] ? $GLOBALS['TYPO3_DB']->fullQuoteStr($config['selectFields'],$table) : '*';

I just need a hint which elements of $queryParts have to be changed, "SELECT" is clear after Jo's hint but I'm not sure about all the other parts.

I'm awaiting your comments ;-)

PS: concerns the functions tslib_cObj::getQuery() and tslib_cObj::getWhere()
Adding patch 8 with changes so far.

#9

Updated by Jo Hasenau about 12 years ago

Well - the current situation already is exactly as I described in my post. It doesn't matter which part of the SQL query you send through stdWrap, as long as the result is not properly escaped, the possibilities are the same.

#10

Updated by tyler kraft about 12 years ago

Hi Joey,

I agree with your concerns regarding security, but at the same time the select property is very limited because of the fact that we can't use stdWrap on many parts of it. For instance at the moment I'd like to be able to use stdWrap to get a variable and use it with the begin property - but I can't and that makes it very frustrating to try to use.

There has to be a way to have security and stdWrap ability? And surely the T3 admin in question should be aware of the fact that there is the possibility for MySQL injections and use intval with stdWrap to prevent this? (This can be mentioned in TSref that they should be doing this).

While not wanting to open up security issues (of which I'll openly admit that I don't know much about) we shouldn't be babysitting because of poor T3 developers? By that token shouldn't we then remove the PHP, PHP_INT, USER and USER_INT content types from the core as they could also introduce security issues? And should we remove the pre/post user functions form stdWrap because they could then open up security issues anywhere?

Just my thoughts, but I could be wrong. I'm sure that there must be a way to make these properties usable with stdWrap but also secure - I just unfortunately can't really contribute as I'm not a PHP guy (but I can't use stdWrap.intval=1 ;-) )

Ty

#11

Updated by tyler kraft almost 12 years ago

Hi

With feature freeze being imminent for 4.3, what are the chances of this being included?

This would be very very helpful if it was, as it would allow a new wider range of uses and flexibility for the select in typoscript.

Tyler

#12

Updated by Ernesto Baschny over 10 years ago

Fixed in #24089 by the stdWrap optimization project. Will be included starting 4.5.0 (beta3).

#13

Updated by Susanne Moog over 10 years ago

  • Target version deleted (4.5.0)

Also available in: Atom PDF