diff --git a/src/main/java/com/alibaba/druid/sql/ast/SQLOrderBy.java b/src/main/java/com/alibaba/druid/sql/ast/SQLOrderBy.java index ee345a2baf..be5c893e27 100644 --- a/src/main/java/com/alibaba/druid/sql/ast/SQLOrderBy.java +++ b/src/main/java/com/alibaba/druid/sql/ast/SQLOrderBy.java @@ -35,6 +35,13 @@ public class SQLOrderBy extends SQLObjectImpl { this.items.add(item); } + public void addItem(SQLSelectOrderByItem item) { + if (item != null) { + item.setParent(this); + } + this.items.add(item); + } + public List getItems() { return this.items; } diff --git a/src/main/java/com/alibaba/druid/sql/ast/statement/SQLSelectGroupByClause.java b/src/main/java/com/alibaba/druid/sql/ast/statement/SQLSelectGroupByClause.java index 5836b52bf3..6e5a71e1a6 100644 --- a/src/main/java/com/alibaba/druid/sql/ast/statement/SQLSelectGroupByClause.java +++ b/src/main/java/com/alibaba/druid/sql/ast/statement/SQLSelectGroupByClause.java @@ -51,4 +51,11 @@ public class SQLSelectGroupByClause extends SQLObjectImpl { public List getItems() { return this.items; } + + public void addItem(SQLExpr sqlExpr) { + if (sqlExpr != null) { + sqlExpr.setParent(this); + this.items.add(sqlExpr); + } + } } diff --git a/src/main/java/com/alibaba/druid/sql/dialect/mysql/ast/statement/MySqlSelectQueryBlock.java b/src/main/java/com/alibaba/druid/sql/dialect/mysql/ast/statement/MySqlSelectQueryBlock.java index ea1b1714b7..161b843c52 100644 --- a/src/main/java/com/alibaba/druid/sql/dialect/mysql/ast/statement/MySqlSelectQueryBlock.java +++ b/src/main/java/com/alibaba/druid/sql/dialect/mysql/ast/statement/MySqlSelectQueryBlock.java @@ -281,6 +281,9 @@ public class MySqlSelectQueryBlock extends SQLSelectQueryBlock implements MySqlO } public void setRowCount(SQLExpr rowCount) { + if (rowCount != null) { + rowCount.setParent(this); + } this.rowCount = rowCount; } @@ -289,6 +292,9 @@ public class MySqlSelectQueryBlock extends SQLSelectQueryBlock implements MySqlO } public void setOffset(SQLExpr offset) { + if (offset != null) { + offset.setParent(this); + } this.offset = offset; } diff --git a/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlSelectParser.java b/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlSelectParser.java index 1c82b1b1ff..62c803821d 100644 --- a/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlSelectParser.java +++ b/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlSelectParser.java @@ -235,7 +235,7 @@ public class MySqlSelectParser extends SQLSelectParser { accept(Token.BY); while (true) { - groupBy.getItems().add(this.exprParser.expr()); + groupBy.addItem(this.exprParser.expr()); if (!(lexer.token() == (Token.COMMA))) { break; } @@ -247,7 +247,9 @@ public class MySqlSelectParser extends SQLSelectParser { acceptIdentifier("ROLLUP"); MySqlSelectGroupBy mySqlGroupBy = new MySqlSelectGroupBy(); - mySqlGroupBy.getItems().addAll(groupBy.getItems()); + for (SQLExpr sqlExpr : groupBy.getItems()) { + mySqlGroupBy.addItem(sqlExpr); + } mySqlGroupBy.setRollUp(true); groupBy = mySqlGroupBy; diff --git a/src/main/java/com/alibaba/druid/sql/dialect/oracle/parser/OracleExprParser.java b/src/main/java/com/alibaba/druid/sql/dialect/oracle/parser/OracleExprParser.java index 1b20713ffb..d4ce172654 100644 --- a/src/main/java/com/alibaba/druid/sql/dialect/oracle/parser/OracleExprParser.java +++ b/src/main/java/com/alibaba/druid/sql/dialect/oracle/parser/OracleExprParser.java @@ -86,6 +86,8 @@ public class OracleExprParser extends SQLExprParser { + + public boolean allowStringAdditive = false; /** @@ -729,11 +731,11 @@ public class OracleExprParser extends SQLExprParser { accept(Token.BY); - orderBy.getItems().add(parseSelectOrderByItem()); + orderBy.addItem(parseSelectOrderByItem()); while (lexer.token() == (Token.COMMA)) { lexer.nextToken(); - orderBy.getItems().add(parseSelectOrderByItem()); + orderBy.addItem(parseSelectOrderByItem()); } return orderBy; diff --git a/src/main/java/com/alibaba/druid/sql/dialect/oracle/parser/OracleSelectParser.java b/src/main/java/com/alibaba/druid/sql/dialect/oracle/parser/OracleSelectParser.java index 6b93b5263c..541a7ace25 100644 --- a/src/main/java/com/alibaba/druid/sql/dialect/oracle/parser/OracleSelectParser.java +++ b/src/main/java/com/alibaba/druid/sql/dialect/oracle/parser/OracleSelectParser.java @@ -244,7 +244,7 @@ public class OracleSelectParser extends SQLSelectParser { OracleSelectQueryBlock queryBlock = new OracleSelectQueryBlock(); if (lexer.token() == Token.SELECT) { lexer.nextToken(); - + if (lexer.token() == Token.COMMENT) { lexer.nextToken(); } @@ -575,9 +575,9 @@ public class OracleSelectParser extends SQLSelectParser { accept(Token.LPAREN); exprParser.exprList(groupingSet.getParameters()); accept(Token.RPAREN); - groupBy.getItems().add(groupingSet); + groupBy.addItem(groupingSet); } else { - groupBy.getItems().add(this.exprParser.expr()); + groupBy.addItem(this.exprParser.expr()); } if (!(lexer.token() == (Token.COMMA))) { @@ -612,9 +612,9 @@ public class OracleSelectParser extends SQLSelectParser { accept(Token.LPAREN); exprParser.exprList(groupingSet.getParameters()); accept(Token.RPAREN); - groupBy.getItems().add(groupingSet); + groupBy.addItem(groupingSet); } else { - groupBy.getItems().add(this.exprParser.expr()); + groupBy.addItem(this.exprParser.expr()); } if (!(lexer.token() == (Token.COMMA))) { diff --git a/src/main/java/com/alibaba/druid/sql/dialect/postgresql/parser/PGExprParser.java b/src/main/java/com/alibaba/druid/sql/dialect/postgresql/parser/PGExprParser.java index 394364c024..8852a0ba60 100644 --- a/src/main/java/com/alibaba/druid/sql/dialect/postgresql/parser/PGExprParser.java +++ b/src/main/java/com/alibaba/druid/sql/dialect/postgresql/parser/PGExprParser.java @@ -50,11 +50,11 @@ public class PGExprParser extends SQLExprParser { accept(Token.BY); - orderBy.getItems().add(parseSelectOrderByItem()); + orderBy.addItem(parseSelectOrderByItem()); while (lexer.token() == (Token.COMMA)) { lexer.nextToken(); - orderBy.getItems().add(parseSelectOrderByItem()); + orderBy.addItem(parseSelectOrderByItem()); } return orderBy; diff --git a/src/main/java/com/alibaba/druid/sql/parser/SQLExprParser.java b/src/main/java/com/alibaba/druid/sql/parser/SQLExprParser.java index c6a7c5bd0b..c9c5e43e08 100644 --- a/src/main/java/com/alibaba/druid/sql/parser/SQLExprParser.java +++ b/src/main/java/com/alibaba/druid/sql/parser/SQLExprParser.java @@ -907,11 +907,11 @@ public class SQLExprParser extends SQLParser { accept(Token.BY); - orderBy.getItems().add(parseSelectOrderByItem()); + orderBy.addItem(parseSelectOrderByItem()); while (lexer.token() == Token.COMMA) { lexer.nextToken(); - orderBy.getItems().add(parseSelectOrderByItem()); + orderBy.addItem(parseSelectOrderByItem()); } return orderBy; diff --git a/src/main/java/com/alibaba/druid/sql/parser/SQLSelectParser.java b/src/main/java/com/alibaba/druid/sql/parser/SQLSelectParser.java index 00de63ff56..303eaafa52 100644 --- a/src/main/java/com/alibaba/druid/sql/parser/SQLSelectParser.java +++ b/src/main/java/com/alibaba/druid/sql/parser/SQLSelectParser.java @@ -243,7 +243,7 @@ public class SQLSelectParser extends SQLParser { SQLSelectGroupByClause groupBy = new SQLSelectGroupByClause(); while (true) { - groupBy.getItems().add(expr()); + groupBy.addItem(expr()); if (lexer.token() != Token.COMMA) { break; } @@ -272,7 +272,7 @@ public class SQLSelectParser extends SQLParser { for (;;) { final SQLSelectItem selectItem = parseSelectItem(); selectList.add(selectItem); - + if (lexer.token() != Token.COMMA) { break; } diff --git a/src/main/java/com/alibaba/druid/wall/WallProvider.java b/src/main/java/com/alibaba/druid/wall/WallProvider.java index 4f88ecebb9..8447569c84 100644 --- a/src/main/java/com/alibaba/druid/wall/WallProvider.java +++ b/src/main/java/com/alibaba/druid/wall/WallProvider.java @@ -651,11 +651,12 @@ public abstract class WallProvider { WallVisitor visitor = createWallVisitor(); if (statementList.size() > 0) { - SQLStatement stmt = statementList.get(0); - try { - stmt.accept(visitor); - } catch (ParserException e) { - violations.add(new SyntaxErrorViolation(e, sql)); + for (SQLStatement stmt : statementList) { + try { + stmt.accept(visitor); + } catch (ParserException e) { + violations.add(new SyntaxErrorViolation(e, sql)); + } } } 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 c13610eb13..ec4ec516fc 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java @@ -248,7 +248,8 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi boolean isTop = WallVisitorUtils.isTopNoneFromSelect(this, x); if (!isTop) { boolean allow = true; - if (WallVisitorUtils.isWhereOrHaving(x) && isDeny(varName)) { + if (isDeny(varName) + && (WallVisitorUtils.isWhereOrHaving(x) || WallVisitorUtils.checkSqlExpr(varExpr))) { allow = false; } @@ -321,7 +322,7 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi boolean isTop = WallVisitorUtils.isTopNoneFromSelect(this, x); if (!isTop) { boolean allow = true; - if (WallVisitorUtils.isWhereOrHaving(x) && isDeny(varName)) { + if (isDeny(varName) && (WallVisitorUtils.isWhereOrHaving(x) || WallVisitorUtils.checkSqlExpr(x))) { allow = false; } 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 9b2c93bdb7..41fa585207 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java @@ -262,7 +262,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements } boolean allow = true; - if (WallVisitorUtils.isWhereOrHaving(x) && isDeny(varName)) { + if (isDeny(varName) && (WallVisitorUtils.isWhereOrHaving(x) || WallVisitorUtils.checkSqlExpr(x))) { 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 0e6c74905d..24b8abf039 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -33,6 +33,7 @@ import com.alibaba.druid.sql.SQLUtils; 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.SQLOrderBy; import com.alibaba.druid.sql.ast.SQLStatement; import com.alibaba.druid.sql.ast.expr.SQLAggregateExpr; import com.alibaba.druid.sql.ast.expr.SQLAllColumnExpr; @@ -97,6 +98,7 @@ import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlDeleteStatement; import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlDescribeStatement; import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlInsertStatement; import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlReplaceStatement; +import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlSelectQueryBlock.Limit; import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlSetCharSetStatement; import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlSetNamesStatement; import com.alibaba.druid.sql.dialect.mysql.ast.statement.MySqlShowGrantsStatement; @@ -1123,6 +1125,31 @@ public class WallVisitorUtils { return false; } + public static boolean checkSqlExpr(SQLExpr x) { // check groupby, orderby, limit + if (x == null) { + return false; + } + + SQLObject obj = x; + for (;;) { + SQLObject parent = obj.getParent(); + + if (parent == null) { + return false; + } + + if (parent instanceof SQLSelectGroupByClause) { + return true; + } else if (parent instanceof SQLOrderBy) { + return true; + } else if (parent instanceof Limit) { + return true; + } + + obj = parent; + } + } + public static boolean isWhereOrHaving(SQLObject x) { if (x == null) { return false; @@ -1593,7 +1620,7 @@ public class WallVisitorUtils { return; } - if (isWhereOrHaving(x)) { + if (isWhereOrHaving(x) || checkSqlExpr(x)) { addViolation(visitor, ErrorCode.FUNCTION_DENY, "deny function : " + methodName, x); } } @@ -2078,7 +2105,7 @@ public class WallVisitorUtils { boolean isTopUpdateStatement = false; boolean isTopInsertStatement = false; SQLObject selectParent = x.getParent(); - while (selectParent instanceof SQLUnionQuery // + while (selectParent instanceof SQLSelectQuery // || selectParent instanceof SQLJoinTableSource // || selectParent instanceof SQLSubqueryTableSource // || selectParent instanceof SQLSelect) { diff --git a/src/main/resources/META-INF/druid/wall/sqlserver/deny-function.txt b/src/main/resources/META-INF/druid/wall/sqlserver/deny-function.txt index 7f388db34b..f89398dda9 100644 --- a/src/main/resources/META-INF/druid/wall/sqlserver/deny-function.txt +++ b/src/main/resources/META-INF/druid/wall/sqlserver/deny-function.txt @@ -9,4 +9,6 @@ openrowset openquery has_dbaccess sp_executesql -pg_sleep \ No newline at end of file +pg_sleep +user +system_user \ No newline at end of file diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest91.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest91.java index 81acb76c06..3b56f8818a 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest91.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest91.java @@ -127,6 +127,30 @@ public class MySqlWallTest91 extends TestCase { } } + public void test_false11() { + WallProvider provider = initWallProvider(); + { + String sql = "select * from messages where id=1 limit (select count(*) from products group by concat(version(),0x27202020,floor(rand(0)*2-1)));"; + Assert.assertFalse(provider.checkValid(sql)); + } + } + + public void test_false12() { + WallProvider provider = initWallProvider(); + { + String sql = "select * from dede_admin where id=1 order by if((ascii(substr(user(),1,1))>95),1,2);"; + Assert.assertFalse(provider.checkValid(sql)); + } + } + + public void test_false13() { + WallProvider provider = initWallProvider(); + { + String sql = "select * from dede_admin where id=1 limit if((ascii(substr(user(),1,1))>95),1,0);"; + Assert.assertFalse(provider.checkValid(sql)); + } + } + public void test_true1() throws Exception { WallProvider provider = initWallProvider(); {