Merge pull request #287 from wenshao/master

improve WallFilter
This commit is contained in:
温高铁
2013-03-31 08:27:30 -07:00
14 changed files with 92 additions and 69 deletions
+1 -1
View File
@@ -16,7 +16,7 @@
<spring.version>3.1.1.RELEASE</spring.version>
<junit.version>4.11</junit.version>
<gpg.skip>false</gpg.skip>
<gpg.skip>true</gpg.skip>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<jdk.version>1.6</jdk.version>
</properties>
@@ -279,7 +279,8 @@ public class ConfigFilter extends FilterAdapter {
}
}
private InputStream getFileAsStream(String filePath) throws FileNotFoundException {
@SuppressWarnings("resource")
private InputStream getFileAsStream(String filePath) throws FileNotFoundException {
InputStream inStream = null;
File file = new File(filePath);
if (file.exists()) {
@@ -16,7 +16,6 @@
package com.alibaba.druid.sql.dialect.mysql.ast.statement;
import com.alibaba.druid.sql.ast.SQLExpr;
import com.alibaba.druid.sql.ast.SQLName;
import com.alibaba.druid.sql.ast.statement.SQLAlterTableItem;
import com.alibaba.druid.sql.ast.statement.SQLColumnDefinition;
import com.alibaba.druid.sql.dialect.mysql.ast.MySqlObjectImpl;
@@ -36,7 +36,6 @@ import com.sun.tools.attach.AgentLoadException;
import com.sun.tools.attach.AttachNotSupportedException;
import com.sun.tools.attach.VirtualMachine;
@SuppressWarnings("restriction")
public class DruidStat {
private static final String LOCAL_CONNECTOR_ADDRESS_PROP = "com.sun.management.jmxremote.localConnectorAddress";
@@ -16,6 +16,7 @@
package com.alibaba.druid.wall;
public interface Violation {
String getMessage();
String toString();
}
@@ -423,16 +423,18 @@ public class WallFilter extends FilterAdapter implements WallFilterMBean {
if (violations.size() > 0) {
violationCount.incrementAndGet();
Violation firstViolation = violations.get(0);
if (isLogViolation()) {
LOG.error("sql injection violation : " + sql);
LOG.error("sql injection violation, " + firstViolation.getMessage() + " : " + sql);
}
if (throwException) {
if (violations.get(0) instanceof SyntaxErrorViolation) {
SyntaxErrorViolation violation = (SyntaxErrorViolation) violations.get(0);
throw new SQLException("sql injection violation : " + sql, violation.getException());
throw new SQLException("sql injection violation, " + firstViolation.getMessage() + " : " + sql,
violation.getException());
} else {
throw new SQLException("sql injection violation : " + sql);
throw new SQLException("sql injection violation, " + firstViolation.getMessage() + " : " + sql);
}
}
}
@@ -252,11 +252,11 @@ public abstract class WallProvider {
public WallCheckResult check(String sql) {
WallContext context = WallContext.current();
if (context != null && !this.dbType.equals(context.getDbType())) {
WallContext.clearContext();
}
WallContext.createIfNotExists(this.dbType);
WallCheckResult result = new WallCheckResult();
@@ -270,7 +270,7 @@ public abstract class WallProvider {
sqlStat.incrementAndGetExecuteCount();
return result;
}
try {
SQLStatementParser parser = createParser(sql);
@@ -280,8 +280,9 @@ public abstract class WallProvider {
parser.parseStatementList(result.getStatementList());
if (parser.getLexer().token() != Token.EOF) {
result.getViolations().add(new IllegalSQLObjectViolation(sql));
final Token lastToken = parser.getLexer().token();
if (lastToken != Token.EOF) {
result.getViolations().add(new IllegalSQLObjectViolation("not terminal sql, token " + lastToken, sql));
return result;
}
} catch (NotAllowCommentException e) {
@@ -303,7 +304,7 @@ public abstract class WallProvider {
}
if (result.getStatementList().size() > 1 && !config.isMultiStatementAllow()) {
result.getViolations().add(new IllegalSQLObjectViolation(sql));
result.getViolations().add(new IllegalSQLObjectViolation("multi-statement not allow", sql));
return result;
}
@@ -314,7 +315,7 @@ public abstract class WallProvider {
try {
stmt.accept(visitor);
} catch (ParserException e) {
result.getViolations().add(new IllegalSQLObjectViolation(sql));
result.getViolations().add(new SyntaxErrorViolation(e, sql));
return result;
}
@@ -165,7 +165,7 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi
@Override
public boolean visit(SQLSelectStatement x) {
if (!config.isSelelctAllow()) {
this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x)));
this.getViolations().add(new IllegalSQLObjectViolation("select not allow", this.toSQL(x)));
return false;
}
@@ -177,7 +177,7 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi
if (x.getRowCount() instanceof SQLNumericLiteralExpr) {
int rowCount = ((SQLNumericLiteralExpr) x.getRowCount()).getNumber().intValue();
if (rowCount == 0) {
this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x)));
this.getViolations().add(new IllegalSQLObjectViolation("limit row 0", this.toSQL(x)));
}
}
return true;
@@ -190,7 +190,7 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi
}
if (config.isVariantCheck() && varName.startsWith("@@")) {
violations.add(new IllegalSQLObjectViolation(toSQL(x)));
violations.add(new IllegalSQLObjectViolation("global variable", toSQL(x)));
}
return false;
@@ -216,7 +216,7 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi
@Override
public boolean visit(MySqlOutFileExpr x) {
if (!config.isSelectIntoOutfileAllow()) {
violations.add(new IllegalSQLObjectViolation(toSQL(x)));
violations.add(new IllegalSQLObjectViolation("into out file not allow", toSQL(x)));
}
return true;
@@ -285,7 +285,7 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi
}
if (!allow) {
violations.add(new IllegalSQLObjectViolation(x.getClass() + " not allow, " + toSQL(x)));
violations.add(new IllegalSQLObjectViolation(x.getClass() + " not allow", toSQL(x)));
}
}
@@ -75,7 +75,7 @@ public class OracleWallVisitor extends OracleASTVisitorAdapter implements WallVi
public void addViolation(Violation violation) {
this.violations.add(violation);
}
public List<Violation> getViolations() {
return violations;
}
@@ -84,7 +84,7 @@ public class OracleWallVisitor extends OracleASTVisitorAdapter implements WallVi
String name = x.getName();
name = WallVisitorUtils.form(name);
if (config.isVariantCheck() && config.getDenyVariants().contains(name)) {
getViolations().add(new IllegalSQLObjectViolation(toSQL(x)));
getViolations().add(new IllegalSQLObjectViolation("variable not allow : " + name, toSQL(x)));
}
return true;
}
@@ -195,17 +195,17 @@ public class OracleWallVisitor extends OracleASTVisitorAdapter implements WallVi
allow = true;
} else if (x instanceof SQLTruncateStatement) {
allow = config.isTruncateAllow();
}
}
if (!allow) {
violations.add(new IllegalSQLObjectViolation(toSQL(x)));
violations.add(new IllegalSQLObjectViolation("not allow statement", toSQL(x)));
}
}
@Override
public boolean visit(SQLSelectStatement x) {
if (!config.isSelelctAllow()) {
this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x)));
this.getViolations().add(new IllegalSQLObjectViolation("select not allow", this.toSQL(x)));
return false;
}
@@ -234,7 +234,7 @@ public class OracleWallVisitor extends OracleASTVisitorAdapter implements WallVi
@Override
public boolean visit(OracleMultiInsertStatement x) {
if (!config.isInsertAllow()) {
this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x)));
this.getViolations().add(new IllegalSQLObjectViolation("insert not allow", this.toSQL(x)));
return false;
}
@@ -263,7 +263,7 @@ public class OracleWallVisitor extends OracleASTVisitorAdapter implements WallVi
return true;
}
@Override
public boolean visit(SQLSelectItem x) {
WallVisitorUtils.check(this, x);
@@ -79,7 +79,7 @@ public class PGWallVisitor extends PGASTVisitorAdapter implements WallVisitor {
String name = x.getName();
name = WallVisitorUtils.form(name);
if (config.isVariantCheck() && config.getDenyVariants().contains(name)) {
getViolations().add(new IllegalSQLObjectViolation(toSQL(x)));
getViolations().add(new IllegalSQLObjectViolation("variable not allow : " + name, toSQL(x)));
}
return true;
}
@@ -186,14 +186,14 @@ public class PGWallVisitor extends PGASTVisitorAdapter implements WallVisitor {
}
if (!allow) {
violations.add(new IllegalSQLObjectViolation(toSQL(x)));
violations.add(new IllegalSQLObjectViolation("not allow statement", toSQL(x)));
}
}
@Override
public boolean visit(SQLSelectStatement x) {
if (!config.isSelelctAllow()) {
this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x)));
this.getViolations().add(new IllegalSQLObjectViolation("selelct not allow", this.toSQL(x)));
return false;
}
@@ -73,7 +73,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements
public void addViolation(Violation violation) {
this.violations.add(violation);
}
@Override
public List<Violation> getViolations() {
return violations;
@@ -97,7 +97,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements
String name = x.getName();
name = WallVisitorUtils.form(name);
if (config.isVariantCheck() && config.getDenyVariants().contains(name)) {
getViolations().add(new IllegalSQLObjectViolation(toSQL(x)));
getViolations().add(new IllegalSQLObjectViolation("variable not allow : " + name, toSQL(x)));
}
return true;
}
@@ -138,7 +138,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements
WallVisitorUtils.checkHaving(this, x.getHaving());
return true;
}
@Override
public boolean visit(SQLServerSelectQueryBlock x) {
WallVisitorUtils.checkSelelct(this, x);
@@ -185,14 +185,14 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements
}
if (!allow) {
violations.add(new IllegalSQLObjectViolation(toSQL(x)));
violations.add(new IllegalSQLObjectViolation("not allow statement", toSQL(x)));
}
}
@Override
public boolean visit(SQLSelectStatement x) {
if (!config.isSelelctAllow()) {
this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x)));
this.getViolations().add(new IllegalSQLObjectViolation("selelct not allow", this.toSQL(x)));
return false;
}
@@ -202,7 +202,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements
@Override
public boolean visit(SQLInsertStatement x) {
if (!config.isInsertAllow()) {
this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x)));
this.getViolations().add(new IllegalSQLObjectViolation("insert not allow", this.toSQL(x)));
return false;
}
@@ -229,7 +229,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements
}
if (config.isVariantCheck() && varName.startsWith("@@")) {
violations.add(new IllegalSQLObjectViolation(toSQL(x)));
violations.add(new IllegalSQLObjectViolation("global variable not allow", toSQL(x)));
}
return false;
@@ -238,14 +238,15 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements
@Override
public boolean visit(SQLServerObjectReferenceExpr x) {
if (x.getSchema() != null && !provider.checkDenySchema(x.getSchema())) {
this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x)));
this.getViolations().add(new IllegalSQLObjectViolation("schema not allow : " + x.getSchema(), this.toSQL(x)));
}
if (x.getDatabase() != null && !provider.checkDenySchema(x.getDatabase())) {
this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x)));
this.getViolations().add(new IllegalSQLObjectViolation("schema not allow : " + x.getDatabase(),
this.toSQL(x)));
}
return true;
}
@Override
public boolean visit(SQLServerInsertStatement x) {
this.visit((SQLInsertStatement) x);
@@ -254,9 +255,9 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements
@Override
public void endVisit(SQLServerInsertStatement x) {
this.endVisit((SQLInsertStatement) x);
this.endVisit((SQLInsertStatement) x);
}
@Override
public boolean visit(SQLSelectItem x) {
WallVisitorUtils.check(this, x);
@@ -93,7 +93,7 @@ public class WallVisitorUtils {
SQLTableSource from = queryBlock.getFrom();
if (from instanceof SQLExprTableSource) {
addViolation(visitor, x);
addViolation(visitor, "'SELECT *' not allow", x);
}
}
}
@@ -106,7 +106,7 @@ public class WallVisitorUtils {
checkReadOnly(visitor, x.getTableSource());
if (!visitor.getConfig().isInsertAllow()) {
addViolation(visitor, x);
addViolation(visitor, "insert not allow", x);
}
}
@@ -120,7 +120,7 @@ public class WallVisitorUtils {
}
if (!visitor.getConfig().isSelectIntoAllow() && x.getInto() != null) {
addViolation(visitor, x);
addViolation(visitor, "select into not allow", x);
return;
}
@@ -144,7 +144,7 @@ public class WallVisitorUtils {
}
if (!isSimpleConstExpr) {
addViolation(visitor, x);
addViolation(visitor, "select alway true condition not allow", x);
}
}
@@ -158,7 +158,7 @@ public class WallVisitorUtils {
}
if (Boolean.TRUE == getConditionValue(visitor, x, visitor.getConfig().isSelectHavingAlwayTrueCheck())) {
addViolation(visitor, x);
addViolation(visitor, "having alway true condition not allow", x);
}
}
@@ -166,14 +166,14 @@ public class WallVisitorUtils {
checkReadOnly(visitor, x.getExprTableSource());
if (!visitor.getConfig().isDeleteAllow()) {
addViolation(visitor, x);
addViolation(visitor, "delete not allow", x);
return;
}
if (x.getWhere() == null
|| Boolean.TRUE == getConditionValue(visitor, x.getWhere(),
visitor.getConfig().isDeleteWhereAlwayTrueCheck())) {
addViolation(visitor, x);
addViolation(visitor, "delete alway true condition not allow", x);
return;
}
@@ -192,7 +192,7 @@ public class WallVisitorUtils {
x.accept(exportParameterVisitor);
if (exportParameterVisitor.getParameters().size() > 0) {
addViolation(visitor, x);
addViolation(visitor, "sql must parameterized", x);
return;
}
}
@@ -343,7 +343,7 @@ public class WallVisitorUtils {
boolean readOnlyValid = visitor.getProvider().checkReadOnlyTable(tableName);
if (!readOnlyValid) {
addViolation(visitor, tableSource);
addViolation(visitor, "table readonly : " + tableName, tableSource);
}
} else if (tableSource instanceof SQLJoinTableSource) {
SQLJoinTableSource join = (SQLJoinTableSource) tableSource;
@@ -357,7 +357,7 @@ public class WallVisitorUtils {
checkReadOnly(visitor, x.getTableSource());
if (!visitor.getConfig().isUpdateAllow()) {
addViolation(visitor, x);
addViolation(visitor, "update not allow", x);
return;
}
@@ -366,17 +366,17 @@ public class WallVisitorUtils {
if (x instanceof MySqlUpdateStatement) {
MySqlUpdateStatement mysqlUpdate = (MySqlUpdateStatement) x;
if (mysqlUpdate.getLimit() == null) {
addViolation(visitor, x);
addViolation(visitor, "update alway true condition not allow", x);
return;
}
} else {
addViolation(visitor, x);
addViolation(visitor, "update alway true condition not allow", x);
return;
}
}
if (Boolean.TRUE == getConditionValue(visitor, x.getWhere(), true)) {
addViolation(visitor, x);
addViolation(visitor, "update alway true condition not allow", x);
return;
}
}
@@ -566,19 +566,19 @@ public class WallVisitorUtils {
final WallConditionContext current = wallConditionContextLocal.get();
if (current.hasPartAlwayTrue() && alwayTrueCheck) {
addViolation(visitor, x);
addViolation(visitor, "part alway true condition not allow", x);
}
if (current.hasConstArithmetic() && !visitor.getConfig().isConstArithmeticAllow()) {
addViolation(visitor, x);
addViolation(visitor, "const arithmetic not allow", x);
}
if (current.hasXor() && !visitor.getConfig().isConditionOpXorAllow()) {
addViolation(visitor, x);
addViolation(visitor, "xor not allow", x);
}
if (current.hasBitwise() && !visitor.getConfig().isConditionOpBitwseAllow()) {
addViolation(visitor, x);
addViolation(visitor, "bitwise operator not allow", x);
}
return value;
@@ -737,7 +737,7 @@ public class WallVisitorUtils {
String methodName = x.getMethodName();
if (!visitor.getProvider().checkDenyFunction(methodName)) {
addViolation(visitor, x);
addViolation(visitor, "deny function : " + methodName, x);
}
}
@@ -747,12 +747,12 @@ public class WallVisitorUtils {
String owner = ((SQLName) x).getSimleName();
owner = WallVisitorUtils.form(owner);
if (!visitor.getProvider().checkDenySchema(owner)) {
addViolation(visitor, x);
addViolation(visitor, "deny schema : " + owner, x);
return false;
}
if (visitor.getConfig().isDenyObjects(owner)) {
addViolation(visitor, x);
addViolation(visitor, "deny object : " + owner, x);
return false;
}
}
@@ -778,7 +778,7 @@ public class WallVisitorUtils {
if (expr instanceof SQLName) {
String tableName = ((SQLName) expr).getSimleName();
if (visitor.isDenyTable(tableName)) {
addViolation(visitor, x);
addViolation(visitor, "deny table : " + tableName, x);
return false;
}
}
@@ -786,18 +786,18 @@ public class WallVisitorUtils {
return true;
}
private static void addViolation(WallVisitor visitor, SQLObject x) {
visitor.addViolation(new IllegalSQLObjectViolation(visitor.toSQL(x)));
private static void addViolation(WallVisitor visitor, String message, SQLObject x) {
visitor.addViolation(new IllegalSQLObjectViolation(message, visitor.toSQL(x)));
}
public static void checkUnion(WallVisitor visitor, SQLUnionQuery x) {
if (x.getOperator() == SQLUnionOperator.MINUS && !visitor.getConfig().isMinusAllow()) {
addViolation(visitor, x);
addViolation(visitor, "minus not allow", x);
return;
}
if (x.getOperator() == SQLUnionOperator.INTERSECT && !visitor.getConfig().isIntersectAllow()) {
addViolation(visitor, x);
addViolation(visitor, "intersect not allow", x);
return;
}
@@ -806,7 +806,7 @@ public class WallVisitorUtils {
}
if (WallVisitorUtils.queryBlockFromIsNull(x.getLeft()) || WallVisitorUtils.queryBlockFromIsNull(x.getRight())) {
addViolation(visitor, x);
addViolation(visitor, "union query not contains 'from clause'", x);
}
}
@@ -19,13 +19,14 @@ import com.alibaba.druid.wall.Violation;
public class IllegalSQLObjectViolation implements Violation {
private String message;
private String sqlPart;
public IllegalSQLObjectViolation(){
}
public IllegalSQLObjectViolation(String condition){
public IllegalSQLObjectViolation(String message, String condition){
this.sqlPart = condition;
}
@@ -41,4 +42,15 @@ public class IllegalSQLObjectViolation implements Violation {
return this.sqlPart;
}
public String getMessage() {
return message;
}
public void setMessage(String message) {
this.message = message;
}
}
@@ -41,4 +41,11 @@ public class SyntaxErrorViolation implements Violation {
return sql;
}
public String getMessage() {
if (exception == null) {
return null;
}
return exception.getMessage();
}
}