From de2a3c8ccae8bc219774a8a249d98a309b77ca16 Mon Sep 17 00:00:00 2001 From: kiki Date: Tue, 22 Oct 2013 14:26:57 +0800 Subject: [PATCH 1/4] sqlwall add partAlwayFalse rule --- .../sql/visitor/SQLEvalVisitorUtils.java | 6 + .../com/alibaba/druid/wall/WallConfig.java | 135 ++++++++++-------- .../druid/wall/spi/WallVisitorUtils.java | 26 +++- .../druid/wall/violation/ErrorCode.java | 1 + 4 files changed, 100 insertions(+), 68 deletions(-) diff --git a/src/main/java/com/alibaba/druid/sql/visitor/SQLEvalVisitorUtils.java b/src/main/java/com/alibaba/druid/sql/visitor/SQLEvalVisitorUtils.java index 0656de218c..10e5359333 100644 --- a/src/main/java/com/alibaba/druid/sql/visitor/SQLEvalVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/sql/visitor/SQLEvalVisitorUtils.java @@ -854,6 +854,12 @@ public class SQLEvalVisitorUtils { wallConditionContext.setPartAlwayTrue(true); } } + } else if (x.getOperator() == SQLBinaryOperator.BooleanAnd) { + if (wallConditionContext != null) { + if (left.getAttribute(EVAL_VALUE) == Boolean.FALSE || right.getAttribute(EVAL_VALUE) == Boolean.FALSE) { + wallConditionContext.setPartAlwayFalse(true); + } + } } else if (x.getOperator() == SQLBinaryOperator.BooleanXor) { if (wallConditionContext != null) { wallConditionContext.setXor(true); diff --git a/src/main/java/com/alibaba/druid/wall/WallConfig.java b/src/main/java/com/alibaba/druid/wall/WallConfig.java index cc6ec4d723..61d05f39b4 100644 --- a/src/main/java/com/alibaba/druid/wall/WallConfig.java +++ b/src/main/java/com/alibaba/druid/wall/WallConfig.java @@ -24,80 +24,81 @@ import com.alibaba.druid.wall.spi.WallVisitorUtils; public class WallConfig implements WallConfigMBean { - private boolean noneBaseStatementAllow = false; + private boolean noneBaseStatementAllow = false; - private boolean callAllow = true; - private boolean selelctAllow = true; - private boolean selectIntoAllow = true; - private boolean selectIntoOutfileAllow = false; - private boolean selectWhereAlwayTrueCheck = true; - private boolean selectHavingAlwayTrueCheck = true; - private boolean selectUnionCheck = true; - private boolean selectMinusCheck = true; - private boolean selectExceptCheck = true; - private boolean selectIntersectCheck = true; - private boolean createTableAllow = true; - private boolean dropTableAllow = true; - private boolean alterTableAllow = true; + private boolean callAllow = true; + private boolean selelctAllow = true; + private boolean selectIntoAllow = true; + private boolean selectIntoOutfileAllow = false; + private boolean selectWhereAlwayTrueCheck = true; + private boolean selectHavingAlwayTrueCheck = true; + private boolean selectUnionCheck = true; + private boolean selectMinusCheck = true; + private boolean selectExceptCheck = true; + private boolean selectIntersectCheck = true; + private boolean createTableAllow = true; + private boolean dropTableAllow = true; + private boolean alterTableAllow = true; - private boolean conditionAndAlwayTrueAllow = false; - private boolean conditionDoubleConstAllow = false; - private boolean conditionLikeTrueAllow = true; + private boolean conditionAndAlwayTrueAllow = false; + private boolean conditionAndAlwayFalseAllow = false; + private boolean conditionDoubleConstAllow = false; + private boolean conditionLikeTrueAllow = true; - private boolean selectAllColumnAllow = true; + private boolean selectAllColumnAllow = true; - private boolean deleteAllow = true; - private boolean deleteWhereAlwayTrueCheck = true; - private boolean deleteWhereNoneCheck = false; + private boolean deleteAllow = true; + private boolean deleteWhereAlwayTrueCheck = true; + private boolean deleteWhereNoneCheck = false; - private boolean updateAllow = true; - private boolean updateWhereAlayTrueCheck = true; - private boolean updateWhereNoneCheck = false; + private boolean updateAllow = true; + private boolean updateWhereAlayTrueCheck = true; + private boolean updateWhereNoneCheck = false; - private boolean insertAllow = true; - private boolean mergeAllow = true; - private boolean minusAllow = true; - private boolean intersectAllow = true; - private boolean replaceAllow = true; - private boolean setAllow = true; - private boolean commitAllow = true; - private boolean rollbackAllow = true; - private boolean useAllow = true; + private boolean insertAllow = true; + private boolean mergeAllow = true; + private boolean minusAllow = true; + private boolean intersectAllow = true; + private boolean replaceAllow = true; + private boolean setAllow = true; + private boolean commitAllow = true; + private boolean rollbackAllow = true; + private boolean useAllow = true; - private boolean multiStatementAllow = false; + private boolean multiStatementAllow = false; - private boolean truncateAllow = true; + private boolean truncateAllow = true; - private boolean commentAllow = false; - private boolean strictSyntaxCheck = true; - private boolean constArithmeticAllow = true; - private boolean limitZeroAllow = false; + private boolean commentAllow = false; + private boolean strictSyntaxCheck = true; + private boolean constArithmeticAllow = true; + private boolean limitZeroAllow = false; - private boolean describeAllow = true; - private boolean showAllow = true; + private boolean describeAllow = true; + private boolean showAllow = true; - private boolean schemaCheck = true; - private boolean tableCheck = true; - private boolean functionCheck = true; - private boolean objectCheck = true; - private boolean variantCheck = true; + private boolean schemaCheck = true; + private boolean tableCheck = true; + private boolean functionCheck = true; + private boolean objectCheck = true; + private boolean variantCheck = true; - private boolean mustParameterized = false; + private boolean mustParameterized = false; - private boolean doPrivilegedAllow = false; + private boolean doPrivilegedAllow = false; - protected final Set denyFunctions = new ConcurrentSkipListSet(); - protected final Set denyTables = new ConcurrentSkipListSet(); - protected final Set denySchemas = new ConcurrentSkipListSet(); - protected final Set denyVariants = new ConcurrentSkipListSet(); - protected final Set denyObjects = new ConcurrentSkipListSet(); + protected final Set denyFunctions = new ConcurrentSkipListSet(); + protected final Set denyTables = new ConcurrentSkipListSet(); + protected final Set denySchemas = new ConcurrentSkipListSet(); + protected final Set denyVariants = new ConcurrentSkipListSet(); + protected final Set denyObjects = new ConcurrentSkipListSet(); - protected final Set permitFunctions = new ConcurrentSkipListSet(); - protected final Set permitTables = new ConcurrentSkipListSet(); - protected final Set permitSchemas = new ConcurrentSkipListSet(); - protected final Set permitVariants = new ConcurrentSkipListSet(); + protected final Set permitFunctions = new ConcurrentSkipListSet(); + protected final Set permitTables = new ConcurrentSkipListSet(); + protected final Set permitSchemas = new ConcurrentSkipListSet(); + protected final Set permitVariants = new ConcurrentSkipListSet(); - protected final Set readOnlyTables = new ConcurrentSkipListSet(); + protected final Set readOnlyTables = new ConcurrentSkipListSet(); private String dir; @@ -106,13 +107,13 @@ public class WallConfig implements WallConfigMBean { private String tenantTablePattern; private String tenantColumn; - private boolean wrapAllow = true; - private boolean metadataAllow = true; + private boolean wrapAllow = true; + private boolean metadataAllow = true; - private boolean conditionOpXorAllow = false; - private boolean conditionOpBitwseAllow = true; + private boolean conditionOpXorAllow = false; + private boolean conditionOpBitwseAllow = true; - private boolean caseConditionAllow = false; + private boolean caseConditionAllow = false; public WallConfig(){ @@ -158,6 +159,14 @@ public class WallConfig implements WallConfigMBean { this.conditionAndAlwayTrueAllow = conditionAndAlwayTrueAllow; } + public boolean isConditionAndAlwayFalseAllow() { + return conditionAndAlwayFalseAllow; + } + + public void setConditionAndAlwayFalseAllow(boolean conditionAndAlwayFalseAllow) { + this.conditionAndAlwayFalseAllow = conditionAndAlwayFalseAllow; + } + public boolean isUseAllow() { return useAllow; } diff --git a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java index b829e8013d..370da33580 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -33,7 +33,6 @@ import com.alibaba.druid.sql.ast.SQLExpr; import com.alibaba.druid.sql.ast.SQLName; import com.alibaba.druid.sql.ast.SQLObject; import com.alibaba.druid.sql.ast.SQLStatement; -import com.alibaba.druid.sql.ast.SQLStatementImpl; import com.alibaba.druid.sql.ast.expr.SQLAggregateExpr; import com.alibaba.druid.sql.ast.expr.SQLAllColumnExpr; import com.alibaba.druid.sql.ast.expr.SQLBetweenExpr; @@ -650,6 +649,10 @@ public class WallVisitorUtils { } dalConst++; } else if (Boolean.FALSE == booleanVal) { + final WallConditionContext wallContext = WallVisitorUtils.getWallConditionContext(); + if (wallContext != null && !isFirst(item)) { + wallContext.setPartAlwayFalse(true); + } allTrue = Boolean.FALSE; dalConst++; } else { @@ -869,17 +872,26 @@ public class WallVisitorUtils { public static class WallConditionContext { - private boolean partAlwayrue; + private boolean partAlwayTrue = false; + private boolean partAlwayFalse = false; private boolean constArithmetic = false; private boolean xor = false; private boolean bitwise = false; public boolean hasPartAlwayTrue() { - return partAlwayrue; + return partAlwayTrue; } public void setPartAlwayTrue(boolean partAllowTrue) { - this.partAlwayrue = partAllowTrue; + this.partAlwayTrue = partAllowTrue; + } + + public boolean hasPartAlwayFalse() { + return partAlwayFalse; + } + + public void setPartAlwayFalse(boolean partAlwayFalse) { + this.partAlwayFalse = partAlwayFalse; } public boolean hasConstArithmetic() { @@ -947,6 +959,10 @@ public class WallVisitorUtils { addViolation(visitor, ErrorCode.ALWAY_TRUE, "part alway true condition not allow", x); } + if (current.hasPartAlwayFalse() && !visitor.getConfig().isConditionAndAlwayFalseAllow()) { + addViolation(visitor, ErrorCode.ALWAY_FALSE, "part alway false condition not allow", x); + } + if (current.hasConstArithmetic() && !visitor.getConfig().isConstArithmeticAllow()) { addViolation(visitor, ErrorCode.CONST_ARITHMETIC, "const arithmetic not allow", x); } @@ -1548,7 +1564,7 @@ public class WallVisitorUtils { if (x instanceof SQLExprTableSource) { x = x.getParent(); - if (x instanceof SQLStatementImpl) { + if (x instanceof SQLStatement) { x = x.getParent(); if (x == null) { return true; diff --git a/src/main/java/com/alibaba/druid/wall/violation/ErrorCode.java b/src/main/java/com/alibaba/druid/wall/violation/ErrorCode.java index 0159fc69db..eeb9add391 100644 --- a/src/main/java/com/alibaba/druid/wall/violation/ErrorCode.java +++ b/src/main/java/com/alibaba/druid/wall/violation/ErrorCode.java @@ -67,6 +67,7 @@ public interface ErrorCode { public final static int EVIL_HINTS = 2110; public final static int EVIL_NAME = 2111; public final static int EVIL_CONCAT = 2112; + public final static int ALWAY_FALSE = 2113; public final static int NOT_PARAMETERIZED = 2200; public final static int MULTI_TENANT = 2201; From f910534378bbbaa3179bb1eb059f58599e3d0a7a Mon Sep 17 00:00:00 2001 From: kiki Date: Tue, 22 Oct 2013 17:03:06 +0800 Subject: [PATCH 2/4] sqlwall add partAlwayFalse testcase --- .../bvt/filter/wall/MySqlWallTest43.java | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest43.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest43.java index 2bd2723367..aeac2c5801 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest43.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest43.java @@ -36,19 +36,40 @@ public class MySqlWallTest43 extends TestCase { Assert.assertFalse(provider.checkValid(// "SELECT COUNT(1) AS count FROM `team` " + // - "WHERE `team_type` = 'normal' AND 1 = 1 AND `city_id` IN (0,10)")); + "WHERE `team_type` = 'normal' AND 1 = 1 AND `city_id` IN (0,10)")); Assert.assertEquals(1, provider.getTableStats().size()); } - + public void test_true() throws Exception { WallProvider provider = new MySqlWallProvider(); provider.getConfig().setConditionAndAlwayTrueAllow(true); - + Assert.assertTrue(provider.checkValid(// - "SELECT COUNT(1) AS count FROM `team` " + // + "SELECT COUNT(1) AS count FROM `team` " + // "WHERE `team_type` = 'normal' AND 1 = 1 AND `city_id` IN (0,10)")); - + + Assert.assertEquals(1, provider.getTableStats().size()); + } + + public void test_false2() throws Exception { + WallProvider provider = new MySqlWallProvider(); + + Assert.assertFalse(provider.checkValid(// + "SELECT COUNT(1) AS count FROM `team` " + // + "WHERE `team_type` = 'normal' AND 1 = 2 AND `city_id` IN (0,10)")); + + Assert.assertEquals(1, provider.getTableStats().size()); + } + + public void test_true2() throws Exception { + WallProvider provider = new MySqlWallProvider(); + provider.getConfig().setConditionAndAlwayFalseAllow(true); + + Assert.assertTrue(provider.checkValid(// + "SELECT COUNT(1) AS count FROM `team` " + // + "WHERE `team_type` = 'normal' AND 1 = 2 AND `city_id` IN (0,10)")); + Assert.assertEquals(1, provider.getTableStats().size()); } } From f38934ea6bcee649592b955aad33781859e4fff4 Mon Sep 17 00:00:00 2001 From: kiki Date: Sat, 9 Nov 2013 16:52:24 +0800 Subject: [PATCH 3/4] imporve sql wall rule --- .../druid/wall/spi/MySqlWallVisitor.java | 10 ++++++- .../druid/wall/spi/SQLServerWallVisitor.java | 7 +++++ .../druid/wall/spi/WallVisitorUtils.java | 29 +++++++++++++++---- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java b/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java index ad4655ff15..c13610eb13 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java @@ -65,6 +65,7 @@ import com.alibaba.druid.wall.WallContext; import com.alibaba.druid.wall.WallProvider; import com.alibaba.druid.wall.WallSqlTableStat; import com.alibaba.druid.wall.WallVisitor; +import com.alibaba.druid.wall.spi.WallVisitorUtils.WallTopStatementContext; import com.alibaba.druid.wall.violation.ErrorCode; import com.alibaba.druid.wall.violation.IllegalSQLObjectViolation; @@ -310,6 +311,13 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi } if (varName.startsWith("@@") && !checkVar(x.getParent(), x.getName())) { + + final WallTopStatementContext topStatementContext = WallVisitorUtils.getWallTopStatementContext(); + if (topStatementContext != null + && (topStatementContext.fromSysSchema() || topStatementContext.fromSysTable())) { + return false; + } + boolean isTop = WallVisitorUtils.isTopNoneFromSelect(this, x); if (!isTop) { boolean allow = true; @@ -346,7 +354,7 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi @Override public boolean visit(MySqlOutFileExpr x) { - if (!config.isSelectIntoOutfileAllow()) { + if (!config.isSelectIntoOutfileAllow() && !WallVisitorUtils.isTopSelectOutFile(x)) { violations.add(new IllegalSQLObjectViolation(ErrorCode.INTO_OUTFILE, "into out file not allow", toSQL(x))); } diff --git a/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java b/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java index ed3d573ddb..9b2c93bdb7 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java @@ -53,6 +53,7 @@ import com.alibaba.druid.wall.Violation; import com.alibaba.druid.wall.WallConfig; import com.alibaba.druid.wall.WallProvider; import com.alibaba.druid.wall.WallVisitor; +import com.alibaba.druid.wall.spi.WallVisitorUtils.WallTopStatementContext; import com.alibaba.druid.wall.violation.ErrorCode; import com.alibaba.druid.wall.violation.IllegalSQLObjectViolation; @@ -254,6 +255,12 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements if (config.isVariantCheck() && varName.startsWith("@@")) { + final WallTopStatementContext topStatementContext = WallVisitorUtils.getWallTopStatementContext(); + if (topStatementContext != null + && (topStatementContext.fromSysSchema() || topStatementContext.fromSysTable())) { + return false; + } + boolean allow = true; if (WallVisitorUtils.isWhereOrHaving(x) && isDeny(varName)) { allow = false; diff --git a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java index 370da33580..2b47abb0c1 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -88,6 +88,7 @@ import com.alibaba.druid.sql.ast.statement.SQLUnionQuery; import com.alibaba.druid.sql.ast.statement.SQLUpdateStatement; import com.alibaba.druid.sql.ast.statement.SQLUseStatement; import com.alibaba.druid.sql.dialect.mysql.ast.expr.MySqlBooleanExpr; +import com.alibaba.druid.sql.dialect.mysql.ast.expr.MySqlOutFileExpr; import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlCommitStatement; import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlDeleteStatement; import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlDescribeStatement; @@ -204,7 +205,7 @@ public class WallVisitorUtils { SQLExpr expr = x.getExpr(); if (expr instanceof SQLVariantRefExpr) { - if (!isTopSelectStatement(expr) && "@".equals(((SQLVariantRefExpr) expr).getName())) { + if (!isTopSelectItem(expr) && "@".equals(((SQLVariantRefExpr) expr).getName())) { addViolation(visitor, ErrorCode.EVIL_NAME, "@ not allow", x); } } @@ -1492,6 +1493,7 @@ public class WallVisitorUtils { } boolean isWhereQueryExpr = false; + boolean isSelectItem = false; do { x = parent; parent = parent.getParent(); @@ -1503,7 +1505,9 @@ public class WallVisitorUtils { } else if (parent instanceof SQLQueryExpr || parent instanceof SQLInSubQueryExpr || parent instanceof SQLExistsExpr) { isWhereQueryExpr = isWhereOrHaving(parent); - } else if (isWhereQueryExpr && parent instanceof SQLSelectQueryBlock) { + } else if (parent instanceof SQLSelectItem) { + isSelectItem = true; + } else if ((isWhereQueryExpr || isSelectItem) && parent instanceof SQLSelectQueryBlock) { if (hasTableSource((SQLSelectQueryBlock) parent)) { return false; } @@ -1574,8 +1578,7 @@ public class WallVisitorUtils { return false; } - private static boolean isTopSelectStatement(SQLObject x) { - + private static boolean isTopSelectItem(SQLObject x) { for (;;) { if ((x.getParent() instanceof SQLExpr) || (x.getParent() instanceof Item)) { x = x.getParent(); @@ -1587,12 +1590,18 @@ public class WallVisitorUtils { if (!(x.getParent() instanceof SQLSelectItem)) { return false; } + SQLSelectItem item = (SQLSelectItem) x.getParent(); - if (!(item.getParent() instanceof SQLSelectQueryBlock)) { + return isTopSelectStatement(item.getParent()); + } + + private static boolean isTopSelectStatement(SQLObject x) { + + if (!(x instanceof SQLSelectQueryBlock)) { return false; } - SQLSelectQueryBlock queryBlock = (SQLSelectQueryBlock) item.getParent(); + SQLSelectQueryBlock queryBlock = (SQLSelectQueryBlock) x; if (!(queryBlock.getParent() instanceof SQLSelect)) { return false; } @@ -1606,6 +1615,14 @@ public class WallVisitorUtils { return stmt.getParent() == null; } + public static boolean isTopSelectOutFile(MySqlOutFileExpr x) { + if (!(x.getParent() instanceof SQLExprTableSource)) { + return false; + } + SQLExprTableSource tableSource = (SQLExprTableSource) x.getParent(); + return isTopSelectStatement(tableSource.getParent()); + } + public static boolean check(WallVisitor visitor, SQLExprTableSource x) { final WallTopStatementContext topStatementContext = wallTopStatementContextLocal.get(); From a5c9434c15a3ba3cfde317b6d863f88acaded3a8 Mon Sep 17 00:00:00 2001 From: kiki Date: Thu, 14 Nov 2013 15:17:51 +0800 Subject: [PATCH 4/4] modify pom version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index c3e53b528e..711d1b0dc9 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ com.alibaba druid - 0.2.27-SNAPSHOT + 1.0.0 jar druid