Bug #20529
closed
stdWrap for TypoSrcrip-select parameters
0%
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
Updated by Oliver Hader over 15 years ago
Concerning stdWrap functionality for groupBy and orderBy there's already a patch pending in the core list as RFC #20508
Updated by David Bruchmann over 15 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.
Updated by David Bruchmann over 15 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.
Updated by David Bruchmann over 15 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
Updated by Jo Hasenau over 15 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.
Updated by David Bruchmann over 15 years ago
Hi Jo,
what do you think about "where", "andWhere" and the different joins referring stdWrap?
Regards
David
Updated by David Bruchmann over 15 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.
Updated by Jo Hasenau over 15 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.
Updated by tyler kraft over 15 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
Updated by tyler kraft about 15 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
Updated by Ernesto Baschny almost 14 years ago
Fixed in #24089 by the stdWrap optimization project. Will be included starting 4.5.0 (beta3).
Updated by