gavinchou commented on code in PR #65987:
URL: https://github.com/apache/doris/pull/65987#discussion_r3643775367
##########
fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java:
##########
@@ -125,11 +128,71 @@ public ConnectorScanPlanProvider getScanPlanProvider() {
return scanPlanProvider;
}
+ // A scheme-less driver_url must be a plain jar file name: letters,
digits, dot, underscore, hyphen.
+ // This intentionally forbids any path separator, so it can never escape
jdbc_drivers_dir.
+ private static final Pattern SAFE_DRIVER_FILE_NAME =
Pattern.compile("^[A-Za-z0-9._-]+\\.jar$");
+
+ /**
+ * Mandatory, non-configurable driver_url security rule. It is invoked from
+ * {@link JdbcConnectorProvider#validateProperties} (and from {@link
#preCreateValidation}),
+ * i.e. from the engine's {@code checkProperties()} hook, which runs only
on the user-facing
+ * CREATE / ALTER CATALOG paths (both guarded by {@code !isReplay}).
Therefore the rule never
+ * runs during metadata/edit-log replay nor at query time, so existing
catalogs are unaffected
+ * and FE startup / follower replay can never be blocked by it.
+ *
+ * <p>The rule cannot be turned off:
+ * <ul>
+ * <li>any {@code ..} path-traversal segment is rejected, for {@code
file://} and {@code http(s)} alike;</li>
+ * <li>a scheme-less driver_url must be a bare jar file name matching
{@code [A-Za-z0-9._-]+.jar}
+ * (no directories, no special characters), which is then resolved
under {@code jdbc_drivers_dir}.</li>
+ * </ul>
+ * Whether a remote/absolute URL is allowed at all remains governed by the
fe.conf-only
+ * {@code jdbc_driver_secure_path} / {@code jdbc_driver_url_white_list}
configs; this rule only
+ * forbids traversal and enforces the bare-name charset.
+ *
+ * <p>Throws {@link IllegalArgumentException} so the engine wraps it into
a {@code DdlException}
+ * (and, on ALTER, triggers the property rollback).
+ */
+ public static void checkDriverUrlSecurityRule(String driverUrl) {
+ if (driverUrl == null || driverUrl.isEmpty()) {
+ return;
+ }
+ // Check traversal on the decoded path so percent-encoded segments
(e.g. %2e%2e) — which the
+ // driver-loading consumers decode — cannot slip a ".." past this rule.
+ String pathToCheck = driverUrl;
+ if (driverUrl.contains("://")) {
Review Comment:
use existing ssrf checker to check?
the checker provided via fe.conf `security_checker_class_name`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
