postgres-range
postgres-range copied to clipboard
improve macros security
currently macros are registered like that:
Builder::macro(
"where{$operatorName}",
fn ($left, $right) => $this->whereRaw(
sprintf('%s %s %s',
$left instanceof Range ? $left->forSql() : $left,
$operator,
$right instanceof Range ? $right->forSql() : $right
)
)
);
https://github.com/belamov/postgres-range/blob/master/src/Macros/QueryBuilderMacros.php#L25
it is not sanitized in any way, so there's potential sql injection scenario, if you're using macros with unvalidated parameters
the better approach is to use PDO's bindings, like so:
Builder::macro(
"where{$operatorName}",
fn ($left, $right) => $this->whereRaw(
"? $operator ?", [
$left instanceof Range ? $left->forSql() : $left,
$right instanceof Range ? $right->forSql() : $right
]
)
);
or
Builder::macro(
"where{$operatorName}",
fn ($left, $right) => $this->where(
$left instanceof Range ? $left->forSql() : $left,
$operator
$right instanceof Range ? $right->forSql() : $right
)
);
macros are tested like this:
$query = "where$macroMethod";
$items = Range::$query($columnName, $rangeObject)->get();
https://github.com/belamov/postgres-range/blob/master/tests/Feature/MacrosTest.php#L59
with current implementation there is no errors and every macro is working as expected
but with whereRaw
and bindings approach we get sql generated like so:
select * from "ranges" where timestamp_range @> '[2020-04-24 06:52:19,2020-04-25 06:52:19)'::tsrange
and Ambiguous function: 7 ERROR
:
operator is not unique: unknown @> unknown
and with where
approach we get sql generated like so:
select * from "ranges" where "timestamp_range" @> '[2020-04-24 06:48:52,2020-04-25 06:48:52)'::tsrange
and Invalid text representation: 7 ERROR
:
malformed range literal: "'[2020-04-24 06:48:52,2020-04-25 06:48:52)'::tsrange"
in both cases range is transformed as expected and if we run this generated queries with sql, there is no errors. so my guess is that somewhere in value binding to pdo statement this parameters are transformed in some way that pdo driver is not correctly casting types
would be great if somebody know what's the issue is and can help me out with this
I have an idea to prevent SQL injection.
In the macro builder:
- wrap
$left
(column name) in double quotes - convert
$right
(range) to aBelamov\PostgresRange\Ranges\DateRange
object if a string was passed.
For example,
$rangeString = '[2020-04-01,2020-05-01)';
Range::whereRangeOverlaps($columnName, $rangeString);
Macro builder uses the Range
class to parse the string into an object.
use Belamov\PostgresRange\Ranges\DateRange;
// current = create from range arguments
$range = new DateRange('2010-01-10', '2010-01-15', '[', ')');
// or create from string with only 1 argument
$range = new DateRange('[2010-01-10,2010-01-15)');
// or using a parse method
$range = new DateRange::parse('[2010-01-10, 2010-01-15)');
The macro builder can then call ->toSql()
and SQL injection attemps will not parse into a range object and throw an exception.
I don't really want to restrict macros api to accept only range objects. and there is additional problem with your approach - those macros (and range operators) are not for date ranges only. there is multiple range types, how would macro know the format that this string should be parsed into? what if we want query integer range overlapping? or numrange
but manually checking params before adding them to query builder is a good idea, much better than nothing, thanks. will dig into this deeper
I'm not exactly sure if this will help, but this works using PDO:
create table tmp (numbers int4range);
insert into tmp values ('[3, 15]');
select * from tmp
where numbers @> :numbers::int4range;
$pdo = DB::connection()->getPdo();
$query = $pdo->prepare("select * from public.tmp where numbers @> :numbers::int4range");
$query->execute(['numbers' => '[3,4)']);
dd($query->fetchAll());