On 1/19/2022 1:15 PM, Vladimir Sitnikov wrote:
If JDBCAppender is removed thre would be a single exception thrown at config time
Once again: there are valid cases when people override `flushBuffer` and implement secure database calls. In other words, they find "buffering" feature useful.
The buffering feature is not at cause. What is at cause is the unprotected JDBC call which uses a simple Statement instead of a PreparedStatement.
If we drop the class altogether, the users would have to rewrite/recompile the code which is harder than just replacing log4j.jar with reload4j.jar. if we keep the class, we could still keep the "drop-in replacement" label which would be very good.
I agree that 100% drop-in compatibility is better than covering 99%. However, we have to be realistic about the means at our disposal.
That is why I suggest we keep the class, and we heal SQL injection.
OK. I don't think fixing SQL injection can be done without heavy surgery. It would be probably be easier and more sane to write a new JDBCAppender from the ground up.
Then there is the case of MDC (%X), NCD (%x) and many other cases which we did not anticipate
We can hard-code things like "we support PatternLayout only", then we know the full set of markers (e.g. everything with %).
But the problem is the use of PatternLayout. We would need to reinvent JDBC PreparedStamement to overcome the vulnerability.
Vladimir
-- Ceki Gülcü Sponsoring SLF4J/logback/reload4j at https://github.com/sponsors/qos-ch