security fixes

pull/692/head
n0nag0n 5 days ago
parent 6718fedf5a
commit 924f231baa

@ -204,10 +204,12 @@ class Engine
$this->set('flight.case_sensitive', false);
$this->set('flight.handle_errors', true);
$this->set('flight.log_errors', false);
$this->set('flight.debug', false);
$this->set('flight.views.path', './views');
$this->set('flight.views.extension', '.php');
$this->set('flight.content_length', true);
$this->set('flight.v2.output_buffering', false);
$this->set('flight.allow_method_override', true);
// Startup configuration
$this->before('start', function () use ($self) {
@ -225,6 +227,8 @@ class Engine
// which causes a lot of problems. This will be removed
// in v4
$self->response()->v2_output_buffering = $this->get('flight.v2.output_buffering');
// Propagate method override setting to Request
$self->request()::$allowMethodOverride = (bool) $self->get('flight.allow_method_override');
});
$this->initialized = true;
@ -678,16 +682,24 @@ class Engine
public function _error(Throwable $e): void
{
$this->triggerEvent('flight.error', $e);
$msg = sprintf(
<<<'HTML'
<h1>500 Internal Server Error</h1>
<h3>%s (%s)</h3>
<pre>%s</pre>
HTML, // phpcs:ignore
$e->getMessage(),
$e->getCode(),
$e->getTraceAsString()
);
if ($this->get('flight.debug') === true) {
$msg = sprintf(
<<<'HTML'
<h1>500 Internal Server Error</h1>
<h3>%s (%s)</h3>
<pre>%s</pre>
HTML, // phpcs:ignore
htmlspecialchars($e->getMessage(), ENT_QUOTES, 'UTF-8'),
$e->getCode(),
htmlspecialchars($e->getTraceAsString(), ENT_QUOTES, 'UTF-8')
);
} else {
if ($this->get('flight.log_errors') === true) {
error_log($e->getMessage() . "\n" . $e->getTraceAsString());
}
$msg = '<h1>500 Internal Server Error</h1>';
}
try {
$this->response()
@ -890,7 +902,7 @@ class Engine
}
// Append base url to redirect url
if ($base !== '/' && strpos($url, '://') === false) {
if ($base !== '/' && strpos($url, '://') === false) {
$url = $base . preg_replace('#/+#', '/', '/' . $url);
}
@ -1001,7 +1013,11 @@ class Engine
int $option = 0
): void {
$json = $encode ? Json::encode($data, $option) : $data;
$callback = $this->request()->query[$param];
$callback = (string) $this->request()->query[$param];
if ($callback !== '' && !preg_match('/^[A-Za-z_$][\w$.]{0,127}$/', $callback)) {
throw new Exception('Invalid JSONP callback name.');
}
$this->response()
->status($code)

@ -50,6 +50,13 @@ class ControllerCommand extends AbstractBaseCommand
return;
}
$controller = basename($controller);
if (!preg_match('/^[A-Za-z_][A-Za-z0-9_]*$/', str_replace('Controller', '', $controller))) {
$io->error('Controller name must contain only letters, numbers, and underscores.', true);
return;
}
if (!preg_match('/Controller$/', $controller)) {
$controller .= 'Controller';
}

@ -55,6 +55,17 @@ class SimplePdo extends PdoWrapper
}
}
/**
* Validates that an SQL identifier (table or column name) is safe for interpolation.
* Throws PDOException on invalid identifier to prevent SQL injection.
*/
protected function requireSafeIdentifier(string $identifier): void
{
if (!preg_match('/^[A-Za-z_][A-Za-z0-9_]*$/', $identifier)) {
throw new PDOException("Unsafe SQL identifier: '$identifier'");
}
}
/**
* Pulls one row from the query
*
@ -319,6 +330,8 @@ class SimplePdo extends PdoWrapper
*/
public function insert(string $table, array $data): string
{
$this->requireSafeIdentifier($table);
// Detect if this is a bulk insert (array of arrays)
$isBulk = isset($data[0]) && is_array($data[0]);
@ -333,6 +346,10 @@ class SimplePdo extends PdoWrapper
$columns = array_keys($firstRow);
$columnCount = count($columns);
foreach ($columns as $col) {
$this->requireSafeIdentifier((string) $col);
}
// Validate all rows have same columns
foreach ($data as $index => $row) {
if (count($row) !== $columnCount) {
@ -363,6 +380,11 @@ class SimplePdo extends PdoWrapper
} else {
// Single insert
$columns = array_keys($data);
foreach ($columns as $col) {
$this->requireSafeIdentifier((string) $col);
}
$placeholders = array_fill(0, count($data), '?');
$sql = sprintf(
@ -396,8 +418,11 @@ class SimplePdo extends PdoWrapper
*/
public function update(string $table, array $data, string $where, array $whereParams = []): int
{
$this->requireSafeIdentifier($table);
$sets = [];
foreach (array_keys($data) as $column) {
$this->requireSafeIdentifier((string) $column);
$sets[] = "$column = ?";
}
@ -426,6 +451,7 @@ class SimplePdo extends PdoWrapper
*/
public function delete(string $table, string $where, array $whereParams = []): int
{
$this->requireSafeIdentifier($table);
$sql = "DELETE FROM $table WHERE $where";
$stmt = $this->runQuery($sql, $whereParams);
return $stmt->rowCount();

@ -137,6 +137,12 @@ class Request
*/
public string $servername;
/**
* Whether to allow HTTP method override via X-HTTP-Method-Override header or _method POST field.
* Controlled by the flight.allow_method_override engine setting.
*/
public static bool $allowMethodOverride = true;
/**
* Stream path for where to pull the request body from
*/
@ -282,10 +288,12 @@ class Request
{
$method = self::getVar('REQUEST_METHOD', 'GET');
if (self::getVar('HTTP_X_HTTP_METHOD_OVERRIDE') !== '') {
$method = self::getVar('HTTP_X_HTTP_METHOD_OVERRIDE');
} elseif (isset($_REQUEST['_method']) === true) {
$method = $_REQUEST['_method'];
if (self::$allowMethodOverride === true) {
if (self::getVar('HTTP_X_HTTP_METHOD_OVERRIDE') !== '') {
$method = self::getVar('HTTP_X_HTTP_METHOD_OVERRIDE');
} elseif (isset($_REQUEST['_method']) === true) {
$method = $_REQUEST['_method'];
}
}
return strtoupper($method);

@ -87,6 +87,14 @@ class EngineTest extends TestCase
public function testHandleException(): void
{
$engine = new Engine();
$this->expectOutputRegex('~\<h1\>500 Internal Server Error\</h1\>~');
$engine->handleException(new Exception('thrown exception message', 20));
}
public function testHandleExceptionDebugMode(): void
{
$engine = new Engine();
$engine->set('flight.debug', true);
$this->expectOutputRegex('~\<h1\>500 Internal Server Error\</h1\>[\s\S]*\<h3\>thrown exception message \(20\)\</h3\>~');
$engine->handleException(new Exception('thrown exception message', 20));
}

Loading…
Cancel
Save