diff --git a/ProcessMaker/Filters/BaseFilter.php b/ProcessMaker/Filters/BaseFilter.php index 009e3d26f8..828e24df23 100644 --- a/ProcessMaker/Filters/BaseFilter.php +++ b/ProcessMaker/Filters/BaseFilter.php @@ -2,6 +2,7 @@ namespace ProcessMaker\Filters; +use Illuminate\Contracts\Database\Query\Expression; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Arr; use Illuminate\Support\Facades\DB; @@ -98,6 +99,8 @@ public function addToQuery(Builder $query): void private function apply($query): void { + $this->validateRawFilterContext(); + if ($valueAliasMethod = $this->valueAliasMethod()) { $this->valueAliasAdapter($valueAliasMethod, $query); } elseif ($this->subjectType === self::TYPE_STAGE) { @@ -175,7 +178,9 @@ private function manuallyAddJsonWhere($query): void $operator = $this->operator(); $value = $this->value(); - if (!is_numeric($value)) { + if ($value instanceof Expression) { + $value = $value->getValue(DB::connection()->getQueryGrammar()); + } elseif (!is_numeric($value)) { $value = DB::connection()->getPdo()->quote($value); } @@ -259,6 +264,34 @@ private function isJsonData() return $this->subjectType === self::TYPE_FIELD && str_starts_with($this->subjectValue, 'data.'); } + private function validateRawFilterContext(): void + { + if (!$this->filteringWithRawValue()) { + return; + } + + if ($this->subjectType !== self::TYPE_FIELD) { + abort(422, 'Raw filters are only supported for fields.'); + } + + if ($this->isJsonData()) { + return; + } + + $allowedFields = [ + 'created_at', + 'updated_at', + 'initiated_at', + 'completed_at', + 'due_at', + 'started_at', + ]; + + if (!in_array($this->subjectValue, $allowedFields, true)) { + abort(422, 'Raw filters are only supported for temporal fields.'); + } + } + private function subject() { if ($this->isJsonData()) { @@ -299,11 +332,7 @@ public function value() return $this->value . '%'; } - if ($this->filteringWithRawValue()) { - return $this->getRawValue(); - } - - return $this->value; + return $this->valueWithRawExpressions($this->value); } abstract protected function valueAliasMethod(); diff --git a/ProcessMaker/Traits/InteractsWithRawFilter.php b/ProcessMaker/Traits/InteractsWithRawFilter.php index 9052f8b03b..15a5c5fd3b 100644 --- a/ProcessMaker/Traits/InteractsWithRawFilter.php +++ b/ProcessMaker/Traits/InteractsWithRawFilter.php @@ -4,10 +4,10 @@ use Illuminate\Contracts\Database\Query\Expression; use Illuminate\Support\Facades\DB; -use Illuminate\Support\Str; - trait InteractsWithRawFilter { + private const MAX_INTERVAL = 365; + private bool $usesRawValue = false; /** @@ -15,7 +15,7 @@ trait InteractsWithRawFilter * * @var array */ - private array $validRawFilterOperators = ['=', '!=', '>', '<', '>=', '<=']; + private array $validRawFilterOperators = ['=', '!=', '>', '<', '>=', '<=', 'between']; /** * Unwrap the raw() and retrieve the string value passed @@ -24,14 +24,30 @@ trait InteractsWithRawFilter */ public function getRawValue(): Expression { - // Get the string equivalent of the raw() filter value - $value = $this->containsRawValue($this->getValue()) ? $this->getValue() : ''; + $value = $this->getValue(); + $matches = []; + $pattern = '/^\s*raw\(\s*NOW\(\)\s*(?:(\+|-)\s*INTERVAL\s+([1-9]\d*)\s+' + . '(SECOND|MINUTE|HOUR|DAY|WEEK|MONTH))?\s*\)\s*$/iD'; + + if (!is_string($value) || preg_match($pattern, $value, $matches) !== 1) { + abort(422, 'Invalid raw filter expression.'); + } + + if (isset($matches[2]) && (int) $matches[2] > self::MAX_INTERVAL) { + abort(422, 'Raw filter interval exceeds the maximum allowed value.'); + } - // Remove the actual row( and ) from the string - $unwrappedRawValue = $this->unwrapRawValue($value); + $expression = 'NOW()'; + if (isset($matches[1])) { + $expression .= sprintf( + ' %s INTERVAL %d %s', + $matches[1], + (int) $matches[2], + strtoupper($matches[3]) + ); + } - // Wrap it in a DB expression and return it - return DB::raw($unwrappedRawValue); + return DB::raw($expression); } /** @@ -43,8 +59,7 @@ public function getRawValue(): Expression */ public function containsRawValue(string $value): bool { - return Str::contains($value, 'raw(') - && Str::endsWith($value, ')'); + return preg_match('/^\s*raw\s*\(/i', $value) === 1; } /** @@ -54,18 +69,19 @@ public function containsRawValue(string $value): bool */ protected function detectRawValue(): void { - $value = $this->getValue(); - - // Sometimes, the value is an array, which likely means - // this filter is set to the use the "between" operator - $value = is_string($value) ? $value : ''; + $values = is_array($this->getValue()) ? $this->getValue() : [$this->getValue()]; + $this->usesRawValue = false; - // Detect if this particular filter includes a raw() value - $this->usesRawValue = $this->containsRawValue($value); + if ($this->operator === 'between' && (!is_array($this->getValue()) || count($values) !== 2)) { + abort(422, 'The between operator requires exactly two values.'); + } - // If so, validate it is being used with a compatible operator - if ($this->usesRawValue) { - $this->validateOperator(); + foreach ($values as $value) { + if (is_string($value) && $this->containsRawValue($value)) { + $this->usesRawValue = true; + $this->validateOperator(); + $this->validateRawValue($value); + } } } @@ -78,9 +94,7 @@ protected function detectRawValue(): void */ protected function unwrapRawValue(string $value): string { - $stripped = Str::after($value, 'raw('); - - return Str::beforeLast($stripped, ')'); + return substr($value, 4, -1); } /** @@ -103,6 +117,43 @@ protected function filteringWithRawValue(): bool return $this->usesRawValue === true; } + protected function valueWithRawExpressions(mixed $value): mixed + { + if (is_array($value)) { + return array_map(fn ($item) => $this->valueWithRawExpressions($item), $value); + } + + if (is_string($value) && $this->containsRawValue($value)) { + return $this->getRawValueFor($value); + } + + return $value; + } + + private function getRawValueFor(string $value): Expression + { + $originalValue = $this->value; + $this->value = $value; + + try { + return $this->getRawValue(); + } finally { + $this->value = $originalValue; + } + } + + private function validateRawValue(string $value): void + { + $originalValue = $this->value; + $this->value = $value; + + try { + $this->getRawValue(); + } finally { + $this->value = $originalValue; + } + } + /** * Validate the operator for this raw() filter * diff --git a/tests/Feature/Api/ProcessRequestsTest.php b/tests/Feature/Api/ProcessRequestsTest.php index 1556b07c90..a7da3e00ac 100644 --- a/tests/Feature/Api/ProcessRequestsTest.php +++ b/tests/Feature/Api/ProcessRequestsTest.php @@ -2,6 +2,7 @@ namespace Tests\Feature\Api; +use PHPUnit\Framework\Attributes\Group as TestGroup; use Faker\Factory as Faker; use Illuminate\Foundation\Testing\WithFaker; use Illuminate\Http\UploadedFile; @@ -23,8 +24,8 @@ /** * Tests routes related to processes / CRUD related methods * - * @group process_tests */ +#[TestGroup('process_tests')] class ProcessRequestsTest extends TestCase { use RequestHelper; @@ -1092,6 +1093,21 @@ public function testAdvancedFilter() $this->assertEquals($hit->id, $json['data'][0]['id']); } + public function testAdvancedFilterRejectsUntrustedRawExpression() + { + $filter = json_encode([ + [ + 'subject' => ['type' => 'Field', 'value' => 'created_at'], + 'operator' => '=', + 'value' => 'raw((SELECT password FROM users LIMIT 1))', + ], + ]); + + $response = $this->apiCall('GET', self::API_TEST_URL, ['advanced_filter' => $filter]); + + $response->assertStatus(422); + } + // Test enableIsActionbyemail function public function testEnableIsActionbyemail() { diff --git a/tests/Feature/Api/TasksTest.php b/tests/Feature/Api/TasksTest.php index 212fc34d19..063b8064f3 100644 --- a/tests/Feature/Api/TasksTest.php +++ b/tests/Feature/Api/TasksTest.php @@ -783,6 +783,21 @@ public function testAdvancedFilter() $this->assertEquals($hitTask->id, $json['data'][0]['id']); } + public function testAdvancedFilterRejectsUntrustedRawExpression() + { + $filter = json_encode([ + [ + 'subject' => ['type' => 'Field', 'value' => 'due_at'], + 'operator' => '=', + 'value' => 'raw((SELECT password FROM users LIMIT 1))', + ], + ]); + + $response = $this->apiCall('GET', '/tasks', ['advanced_filter' => $filter]); + + $response->assertStatus(422); + } + public function testAdvancedFilterByProcessRequestName() { $hitProcess = Process::factory()->create(['name' => 'foo']); diff --git a/tests/unit/ProcessMaker/FilterTest.php b/tests/unit/ProcessMaker/FilterTest.php index b7c93c3fba..68ad23e729 100644 --- a/tests/unit/ProcessMaker/FilterTest.php +++ b/tests/unit/ProcessMaker/FilterTest.php @@ -54,6 +54,64 @@ public function testRawValue() ); } + public function testRawIntervalValue() + { + $sql = $this->filter([ + [ + 'subject' => ['type' => 'Field', 'value' => 'due_at'], + 'operator' => '<=', + 'value' => 'raw(NOW() + INTERVAL 1 DAY)', + ], + ], ProcessRequestToken::class); + + $this->assertStringContainsString('`due_at` <= NOW() + INTERVAL 1 DAY', $sql); + } + + public function testRawBetweenValues() + { + $sql = $this->filter([ + [ + 'subject' => ['type' => 'Field', 'value' => 'due_at'], + 'operator' => 'between', + 'value' => ['raw(NOW())', 'raw(NOW() + INTERVAL 1 DAY)'], + ], + ], ProcessRequestToken::class); + + $this->assertStringContainsString( + '`due_at` between NOW() and NOW() + INTERVAL 1 DAY', + $sql + ); + } + + public function testRejectsUntrustedRawExpression() + { + try { + $this->filter([ + [ + 'subject' => ['type' => 'Field', 'value' => 'due_at'], + 'operator' => '=', + 'value' => 'raw((SELECT password FROM users LIMIT 1))', + ], + ], ProcessRequestToken::class); + $this->fail('Expected a 422 HttpException for an invalid raw expression.'); + } catch (HttpException $e) { + $this->assertEquals(422, $e->getStatusCode()); + } + } + + public function testAllowsRawExpressionForJsonField() + { + $sql = $this->filter([ + [ + 'subject' => ['type' => 'Field', 'value' => 'data.expiration_date'], + 'operator' => '>=', + 'value' => 'raw(NOW())', + ], + ]); + + $this->assertStringContainsString('NOW()', $sql); + } + public function testCompareDataInteger() { $filter = [