fix(oracle): harden date literal escaping#18216
Conversation
ephys
left a comment
There was a problem hiding this comment.
Even though this PR tries to harden it, this is still a massive security hole and the approach should be changed asap
| @@ -117,7 +117,7 @@ export class OracleDialect extends AbstractDialect<OracleDialectOptions, OracleC | |||
|
|
|||
| escapeString(val: string): string { | |||
| if (val.startsWith('TO_TIMESTAMP_TZ') || val.startsWith('TO_DATE')) { | |||
There was a problem hiding this comment.
Why do we need this at all? Any exception to string escapes is extremely dangerous
This must be removed asap. All user-provided strings go through this. They'll be able to call TO_TIMESTAMP_TZ/TO_DATE on anything
| } | ||
|
|
||
| return this.escape(date); | ||
| return `TO_DATE('${formattedDate}','${format}')`; |
There was a problem hiding this comment.
toBindableValue should never return a function call like this, this makes no sense. It's only supposed to return a value. The return value of toBindableValue is meant to be escaped
The SQL must be added to getBindParamSql and escape
|
@ephys Thanks for the review! I agree that this is not a nice solution, but with how DataTypes are implemented I didn't see another easy fix back in the day. Maybe you can take a look at it from scratch to see how we can work around the requirement of Oracle to have that |
|
I can definitely take a look :) |
Pull Request Checklist
Description of Changes
This PR simplifies the workaround done in
escapeStringfor oracle. It also adds additional unit tests, although we'll move them to the oracle package later.