Escape dynamic SQL identifiers across storage backends

AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/userstorage/sql/UserTable.java

@@ -24,6 +24,12 @@
 import com.bencodez.simpleapi.sql.sqlite.db.SQLite;
 
 public class UserTable extends com.bencodez.simpleapi.sql.sqlite.Table {
+	static String quoteIdentifier(String identifier) {
+		if (identifier == null) {
+			throw new IllegalArgumentException("SQL identifier cannot be null");
+		}
+		return "`" + identifier.replace("`", "``") + "`";
+	}
 
 	private List<Column> columns = new ArrayList<>();
 	private String name;
@@ -194,7 +200,7 @@ public void addColoumn(Column column) {
 			return;
 		}
 		try {
-			String query = "ALTER TABLE " + getName() + " ADD COLUMN `" + column.getName() + "` "
+			String query = "ALTER TABLE " + quoteIdentifier(getName()) + " ADD COLUMN " + quoteIdentifier(column.getName()) + " "
 					+ column.getDataType().toString();
 			PreparedStatement s = sqLite.getSQLConnection().prepareStatement(query);
 			s.executeUpdate();
@@ -210,7 +216,7 @@ public void addColoumn(UserDataKey column) {
 			return;
 		}
 		try {
-			String query = "ALTER TABLE " + getName() + " ADD COLUMN `" + column.getKey() + "` "
+			String query = "ALTER TABLE " + quoteIdentifier(getName()) + " ADD COLUMN " + quoteIdentifier(column.getKey()) + " "
 					+ column.getColumnType();
 			PreparedStatement s = sqLite.getSQLConnection().prepareStatement(query);
 			s.executeUpdate();
@@ -283,7 +289,8 @@ public boolean containsKey(String index) {
 	public void copyColumnData(String columnFromName, String columnToName, DataType dataType) {
 		checkColumn(new Column(columnToName, dataType));
 		checkColumn(new Column(columnFromName, dataType));
-		String sql = "UPDATE `" + getName() + "` SET `" + columnToName + "` = `" + columnFromName + "`;";
+		String sql = "UPDATE " + quoteIdentifier(getName()) + " SET " + quoteIdentifier(columnToName) + " = "
+				+ quoteIdentifier(columnFromName) + ";";
 		try {
 			PreparedStatement s = sqLite.getSQLConnection().prepareStatement(sql);
 			s.executeUpdate();
@@ -489,7 +496,7 @@ public ArrayList<String> getNames() {
 
 	public ArrayList<Integer> getNumbersInColumn(String column) {
 		ArrayList<Integer> result = new ArrayList<>();
-		String sqlStr = "SELECT " + column + " FROM " + getName() + ";";
+		String sqlStr = "SELECT " + quoteIdentifier(column) + " FROM " + quoteIdentifier(getName()) + ";";
 
 		try {
 			PreparedStatement s = sqLite.getSQLConnection().prepareStatement(sqlStr);
@@ -784,7 +791,8 @@ private void bindValue(PreparedStatement statement, int parameter, Column column
 
 	public void wipeColumnData(String columnName, DataType dataType) {
 		checkColumn(new Column(columnName, dataType));
-		String sql = "UPDATE " + getName() + " SET " + columnName + " = " + dataType.getNoValue() + ";";
+		String sql = "UPDATE " + quoteIdentifier(getName()) + " SET " + quoteIdentifier(columnName) + " = "
+				+ dataType.getNoValue() + ";";
 		try {
 			PreparedStatement s = sqLite.getSQLConnection().prepareStatement(sql);
 			s.executeUpdate();

AdvancedCore/src/main/java/com/bencodez/advancedcore/bungeeapi/globaldata/GlobalMySQL.java

@@ -966,7 +966,7 @@ private Object toSqlValue(String column, DataValue value) {
 	public void wipeColumnData(String columnName) {
 		checkColumn(columnName, DataType.STRING);
 
-		String colName = (dbType() == DbType.POSTGRESQL) ? quoteIdent(dbType(), columnName.toLowerCase()) : columnName;
+		String colName = (dbType() == DbType.POSTGRESQL) ? quoteIdent(dbType(), columnName.toLowerCase()) : qi(columnName);
 
 		String sql = "UPDATE " + getName() + " SET " + colName + " = NULL;";
 		try {

AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserTablePreparedStatementTest.java

@@ -3,11 +3,11 @@
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.doNothing;
-import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.doReturn;
 
 import java.sql.Connection;
 import java.sql.PreparedStatement;
@@ -82,4 +82,35 @@ void getUuidBindsPlayerName() throws Exception {
 		verify(statement).setString(1, "O'Brien");
 		verify(statement).executeQuery();
 	}
+
+	@Test
+	void addColumnEscapesAttackerControlledIdentifier() throws Exception {
+		Column primaryKey = new Column("uuid", new DataValueString(UUID));
+		UserTable table = spy(new UserTable(mock(AdvancedCorePlugin.class), "Users",
+				Collections.singletonList(primaryKey), primaryKey));
+		table.setSqLite(sqlite);
+		doReturn(false).when(table).hasColumn(any(Column.class));
+
+		String sql = "ALTER TABLE `Users` ADD COLUMN `name``; DROP TABLE Users;--` STRING";
+		when(connection.prepareStatement(sql)).thenReturn(statement);
+		table.addColoumn(new Column("name`; DROP TABLE Users;--", new DataValueString("value")));
+
+		verify(connection).prepareStatement(sql);
+		verify(statement).executeUpdate();
+	}
+
+	@Test
+	void wipeColumnEscapesIdentifier() throws Exception {
+		Column primaryKey = new Column("uuid", new DataValueString(UUID));
+		UserTable table = spy(new UserTable(mock(AdvancedCorePlugin.class), "Users",
+				Collections.singletonList(primaryKey), primaryKey));
+		table.setSqLite(sqlite);
+		doNothing().when(table).checkColumn(any(Column.class));
+		String sql = "UPDATE `Users` SET `points``; DROP TABLE Users;--` = 0;";
+		when(connection.prepareStatement(sql)).thenReturn(statement);
+
+		table.wipeColumnData("points`; DROP TABLE Users;--", com.bencodez.simpleapi.sql.DataType.INTEGER);
+		verify(connection).prepareStatement(sql);
+		verify(statement).executeUpdate();
+	}
 }