fix/ escape SQL values#627
Conversation
There was a problem hiding this comment.
It would be cleaner to refactor the code to replace doQuery() with the query builder.
$result = $DB->request($criteria);
| ), | ||
| ]; | ||
| } | ||
| unset($_FILES['filename']); |
There was a problem hiding this comment.
Why ?
I'm not sure if this is directly related to the topic of this PR?!
There was a problem hiding this comment.
After move_uploaded_file(), the temporary file is deleted, but $_FILES['filename'] still contains its path.
Later, when rendering the page, Html::footer() calls createFromGlobals(). An UploadedFile instance is created for each entry, and it checks whether the temporary file still exists. Since the file has already been moved and no longer exists, an exception is thrown.
unset() clears the corresponding entry from $_FILES after the file has been moved.
| //Add additional parameters specific to this itemtype (or function checkPresent exists) | ||
| if (method_exists($injectionClass, 'checkPresent')) { | ||
| $where .= $injectionClass->checkPresent($this->values, $options); | ||
| $extra = trim((string) $injectionClass->checkPresent($this->values, $options)); |
There was a problem hiding this comment.
The three implementations of checkPresent() (softwareversion, softwarelicense, networkport) still return raw SQL with unescaped name values.
QueryExpression continues to receive potentially dangerous values.
These methods should either be updated to return an array of criteria, or at the very least, $DB->escape() should be added to the string fields.
2 participants
Checklist before requesting a review
Please delete options that are not relevant.
Description