Skip to content

Commit 81929ad

Browse files
committed
Fix SQL injection vulnerability
Do not read column definitions from client (which included field names passed to the database). Instead pass them in the controller. To make ordering/filtering capabilities of DataTables work, find() needs to be called with the column definition array as a fourth argument. It is not needed for the use of DataTables without ordering/filtering or without server-side processing (AJAX fetches coming from DataTables).
1 parent 47737db commit 81929ad

File tree

2 files changed

+79
-43
lines changed

2 files changed

+79
-43
lines changed

src/Controller/Component/DataTablesComponent.php

Lines changed: 75 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use Cake\Collection\Collection;
55
use Cake\Controller\Component;
66
use Cake\Core\Configure;
7+
use Cake\Network\Exception\BadRequestException;
78
use Cake\ORM\Query;
89
use Cake\ORM\Table;
910
use Cake\ORM\TableRegistry;
@@ -77,104 +78,135 @@ private function _draw()
7778
* Process query data of ajax request regarding order
7879
* Alters $options if delegateOrder is set
7980
* In this case, the model needs to handle the 'customOrder' option.
80-
* @param $options: Query options from the request
81+
* @param $options: Query options
82+
* @param $columns: Column definitions
8183
*/
82-
private function _order(array &$options)
84+
private function _order(array &$options, array &$columns)
8385
{
8486
if (empty($this->request->query['order']))
8587
return;
8688

87-
// -- add custom order
8889
$order = $this->config('order');
89-
foreach($this->request->query['order'] as $item) {
90-
$order[$this->request->query['columns'][$item['column']]['name']] = $item['dir'];
90+
91+
/* extract custom ordering from request */
92+
foreach ($this->request->query['order'] as $item) {
93+
if (empty($columns))
94+
throw new \InvalidArgumentException('Column ordering requested, but no column definitions provided.');
95+
96+
$dir = strtoupper($item['dir']);
97+
if (!in_array($dir, ['ASC', 'DESC']))
98+
throw new BadRequestException('Malformed order direction.');
99+
100+
$c = $columns[$item['column']] ?? null;
101+
if (!$c || !($c['orderable'] ?? true)) // orderable is true by default
102+
throw new BadRequestException('Illegal column ordering.');
103+
104+
if (empty($c['field']))
105+
throw new \InvalidArgumentException('Column description misses field name.');
106+
107+
$order[$c['field']] = $dir;
91108
}
109+
110+
/* apply ordering */
92111
if (!empty($options['delegateOrder'])) {
93112
$options['customOrder'] = $order;
94113
} else {
95114
$this->config('order', $order);
96115
}
97116

98-
// -- remove default ordering as we have a custom one
117+
/* remove default ordering in favor of our custom one */
99118
unset($options['order']);
100119
}
101120

102121
/**
103122
* Process query data of ajax request regarding filtering
104123
* Alters $options if delegateSearch is set
105124
* In this case, the model needs to handle the 'globalSearch' option.
106-
* @param $options: Query options from the request
107-
* @return: returns true if additional filtering takes place
125+
* @param $options: Query options
126+
* @param $columns: Column definitions
127+
* @return: true if additional filtering takes place
108128
*/
109-
private function _filter(array &$options) : bool
129+
private function _filter(array &$options, array &$columns) : bool
110130
{
111-
// -- add limit
131+
/* add limit and offset */
112132
if (!empty($this->request->query['length'])) {
113133
$this->config('length', $this->request->query['length']);
114134
}
115-
116-
// -- add offset
117135
if (!empty($this->request->query['start'])) {
118136
$this->config('start', (int)$this->request->query['start']);
119137
}
120138

121-
// -- don't support any search if columns data missing
122-
if (empty($this->request->query['columns']))
123-
return false;
124-
125-
// -- check table search field
126139
$haveFilters = false;
140+
$delegateSearch = !empty($options['delegateSearch']);
141+
142+
/* add global filter (general search field) */
127143
$globalSearch = $this->request->query['search']['value'] ?? false;
144+
if ($globalSearch) {
145+
if (empty($columns))
146+
throw new \InvalidArgumentException('Filtering requested, but no column definitions provided.');
128147

129-
// -- if delegate option set, defer search to the model and bail out
130-
if (!empty($options['delegateSearch'])) {
131-
if ($globalSearch) {
148+
if ($delegateSearch) {
132149
$options['globalSearch'] = $globalSearch;
133150
$haveFilters = true;
134-
}
135-
foreach ($this->request->query['columns'] as $column) {
136-
$localSearch = $column['search']['value'];
137-
if (!empty($localSearch)) {
138-
$options['localSearch'][$column['name']] = $localSearch;
151+
} else {
152+
foreach ($columns as $c) {
153+
if (!($c['searchable'] ?? true)) // searchable is true by default
154+
continue;
155+
156+
if (empty($c['field']))
157+
throw new \InvalidArgumentException('Column description misses field name.');
158+
159+
$this->_addCondition($c['field'], $globalSearch, 'or');
139160
$haveFilters = true;
140161
}
141162
}
142-
return $haveFilters;
143163
}
144164

145-
// -- add conditions for both table-wide and column search fields
146-
foreach ($this->request->query['columns'] as $column) {
147-
if ($globalSearch && $column['searchable'] == 'true') {
148-
$this->_addCondition($column['name'], $globalSearch, 'or');
149-
$haveFilters = true;
150-
}
151-
$localSearch = $column['search']['value'];
165+
/* add local filters (column search fields) */
166+
foreach ($this->request->query['columns'] ?? [] as $index => $column) {
167+
$localSearch = $column['search']['value'] ?? null;
152168
if (!empty($localSearch)) {
153-
$this->_addCondition($column['name'], $column['search']['value']);
169+
if (empty($columns))
170+
throw new \InvalidArgumentException('Filtering requested, but no column definitions provided.');
171+
172+
$c = $columns[$index] ?? null;
173+
if (!$c || !($c['searchable'] ?? true)) // searchable is true by default
174+
throw new BadRequestException('Illegal filter request.');
175+
176+
if (empty($c['field']))
177+
throw new \InvalidArgumentException('Column description misses field name.');
178+
179+
if ($delegateSearch) {
180+
$options['localSearch'][$c['field']] = $localSearch;
181+
} else {
182+
$this->_addCondition($c['field'], $localSearch);
183+
}
154184
$haveFilters = true;
155185
}
156186
}
187+
157188
return $haveFilters;
158189
}
159190

160191
/**
161192
* Find data
162193
*
163-
* @param $tableName
164-
* @param $finder
165-
* @param array $options
166-
* @return array|\Cake\ORM\Query
194+
* @param $tableName: ORM table name
195+
* @param $finder: Finder name (as in Table::find())
196+
* @param array $options: Finder options (as in Table::find())
197+
* @param array $columns: Column definitions needed for filter/order operations
198+
* @return Query to be evaluated (Query::count() may have already been called)
167199
*/
168-
public function find($tableName, $finder = 'all', array $options = [])
200+
public function find(string $tableName, string $finder = 'all', array $options = [], array $columns = []) : Query
169201
{
170-
$delegateSearch = !empty($options['delegateSearch']);
202+
$delegateSearch = $options['delegateSearch'] ?? false;
171203

172204
// -- get table object
173205
$this->_table = TableRegistry::get($tableName);
174206

175207
// -- process draw & ordering options
176208
$this->_draw();
177-
$this->_order($options);
209+
$this->_order($options, $columns);
178210

179211
// -- call table's finder w/o filters
180212
$data = $this->_table->find($finder, $options);
@@ -183,10 +215,10 @@ public function find($tableName, $finder = 'all', array $options = [])
183215
$this->_viewVars['recordsTotal'] = $data->count();
184216

185217
// -- process filter options
186-
$filters = $this->_filter($options);
218+
$haveFilters = $this->_filter($options, $columns);
187219

188220
// -- apply filters
189-
if ($filters) {
221+
if ($haveFilters) {
190222
if ($delegateSearch) {
191223
// call finder again to process filters (provided in $options)
192224
$data = $this->_table->find($finder, $options);

src/View/Helper/DataTablesHelper.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ public function draw(string $selector, array $options = [])
9898
// fill-in missing language options, in case some were customized
9999
$options['language'] += $this->config('language');
100100

101+
// remove field names, which are an internal/server-side setting
102+
foreach ($options['columns'] as $name => $column)
103+
unset($options['columns'][$name]['field']);
104+
101105
// prepare javascript object from the config, including method calls
102106
$json = CallbackFunction::resolve(json_encode($options));
103107

0 commit comments

Comments
 (0)