mirror of
https://gitee.com/pnoker/iot-dc3.git
synced 2026-09-01 15:33:10 +08:00
security(driver): bind jdbc write value as a parameter to prevent sql injection
The shared JDBC driver base interpolated the point value into the write SQL via
writeQuery.replace("${value}", ...), allowing a crafted write command value to
alter the statement. Bind the value with PreparedStatement#setString against a
single ? placeholder instead. Update the mysql/oracle/postgresql/sqlserver
writeQuery attribute docs to the ? convention and add the module's first test.
This commit is contained in:
@@ -77,6 +77,13 @@
|
||||
<artifactId>dc3-common-driver</artifactId>
|
||||
</dependency>
|
||||
|
||||
<!-- Test -->
|
||||
<dependency>
|
||||
<groupId>org.springframework.boot</groupId>
|
||||
<artifactId>spring-boot-starter-test</artifactId>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
|
||||
</dependencies>
|
||||
|
||||
</project>
|
||||
|
||||
+12
-5
@@ -161,10 +161,10 @@ public abstract class AbstractJdbcDriverCustomService implements DriverCustomSer
|
||||
public Boolean write(Map<String, AttributeBO> driverConfig, Map<String, AttributeBO> pointConfig,
|
||||
DeviceBO device, PointBO point, WritePointValue writePointValue) {
|
||||
String writeQuery = getRequiredConfig(pointConfig, "writeQuery");
|
||||
String resolvedQuery = writeQuery.replace("${value}", writePointValue.getValue(String.class));
|
||||
String value = writePointValue.getValue(String.class);
|
||||
HikariDataSource ds = getConnector(device.getId(), driverConfig);
|
||||
try {
|
||||
return executeWriteQuery(ds, resolvedQuery);
|
||||
return executeWriteQuery(ds, writeQuery, value);
|
||||
} catch (WritePointException e) {
|
||||
invalidateConnector(device.getId(), ds);
|
||||
throw e;
|
||||
@@ -240,16 +240,23 @@ public abstract class AbstractJdbcDriverCustomService implements DriverCustomSer
|
||||
}
|
||||
|
||||
/**
|
||||
* Execute a write SQL query (UPDATE/INSERT/DELETE).
|
||||
* Execute a write SQL query (UPDATE/INSERT/DELETE), binding the point value as a parameter.
|
||||
* <p>
|
||||
* The {@code writeQuery} must use a single {@code ?} placeholder for the written value; the
|
||||
* value is bound via {@link PreparedStatement#setString} rather than concatenated into the SQL,
|
||||
* so a malicious point value cannot alter the statement (no SQL injection).
|
||||
* </p>
|
||||
*
|
||||
* @param ds active HikariDataSource
|
||||
* @param writeQuery SQL write query
|
||||
* @param writeQuery SQL write query containing a single {@code ?} value placeholder
|
||||
* @param value point value to bind to the placeholder
|
||||
* @return true if at least one row was affected
|
||||
* @throws WritePointException if query execution fails
|
||||
*/
|
||||
protected boolean executeWriteQuery(HikariDataSource ds, String writeQuery) {
|
||||
protected boolean executeWriteQuery(HikariDataSource ds, String writeQuery, String value) {
|
||||
try (Connection conn = ds.getConnection();
|
||||
PreparedStatement ps = conn.prepareStatement(writeQuery)) {
|
||||
ps.setString(1, value);
|
||||
int rows = ps.executeUpdate();
|
||||
log.debug("Driver SQL write executed, protocol={}, rows={}", driverCode, rows);
|
||||
return rows > 0;
|
||||
|
||||
+130
@@ -0,0 +1,130 @@
|
||||
/*
|
||||
* Copyright 2016-present the IoT DC3 original author or authors.
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU Affero General Public License as
|
||||
* published by the Free Software Foundation, either version 3 of the
|
||||
* License, or (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU Affero General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU Affero General Public License
|
||||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
package io.github.pnoker.common.sql;
|
||||
|
||||
import com.zaxxer.hikari.HikariDataSource;
|
||||
import io.github.pnoker.common.driver.entity.bean.WritePointValue;
|
||||
import io.github.pnoker.common.driver.entity.bo.AttributeBO;
|
||||
import io.github.pnoker.common.driver.entity.bo.DeviceBO;
|
||||
import io.github.pnoker.common.driver.entity.bo.PointBO;
|
||||
import io.github.pnoker.common.driver.service.DriverSenderService;
|
||||
import io.github.pnoker.common.enums.AttributeTypeEnum;
|
||||
import io.github.pnoker.common.enums.PointTypeEnum;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.junit.jupiter.MockitoExtension;
|
||||
|
||||
import java.sql.Connection;
|
||||
import java.sql.PreparedStatement;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
@ExtendWith(MockitoExtension.class)
|
||||
class AbstractJdbcDriverCustomServiceTest {
|
||||
|
||||
@Mock
|
||||
private DriverSenderService driverSenderService;
|
||||
|
||||
@Mock
|
||||
private HikariDataSource dataSource;
|
||||
|
||||
@Mock
|
||||
private Connection connection;
|
||||
|
||||
@Mock
|
||||
private PreparedStatement preparedStatement;
|
||||
|
||||
private TestJdbcDriver service;
|
||||
|
||||
private static Map<String, AttributeBO> pointConfig(String writeQuery) {
|
||||
Map<String, AttributeBO> m = new HashMap<>();
|
||||
m.put("writeQuery", AttributeBO.builder().value(writeQuery).type(AttributeTypeEnum.STRING).build());
|
||||
return m;
|
||||
}
|
||||
|
||||
private static DeviceBO device(Long id) {
|
||||
DeviceBO device = new DeviceBO();
|
||||
device.setId(id);
|
||||
return device;
|
||||
}
|
||||
|
||||
private static PointBO point(Long id) {
|
||||
PointBO point = new PointBO();
|
||||
point.setId(id);
|
||||
return point;
|
||||
}
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
service = new TestJdbcDriver(driverSenderService);
|
||||
service.initial();
|
||||
// Pre-populate the connection cache so write() reuses the mocked data source
|
||||
// instead of building a real HikariCP pool.
|
||||
service.connectMap.put(1L, dataSource);
|
||||
}
|
||||
|
||||
@Test
|
||||
void writeBindsValueAsParameterRatherThanInterpolating() throws Exception {
|
||||
String writeQuery = "UPDATE sensor SET v = ? WHERE id = 1";
|
||||
String maliciousValue = "1'); DROP TABLE sensor; --";
|
||||
when(dataSource.getConnection()).thenReturn(connection);
|
||||
when(connection.prepareStatement(writeQuery)).thenReturn(preparedStatement);
|
||||
when(preparedStatement.executeUpdate()).thenReturn(1);
|
||||
|
||||
Boolean result = service.write(new HashMap<>(), pointConfig(writeQuery), device(1L), point(1L),
|
||||
WritePointValue.builder().value(maliciousValue).type(PointTypeEnum.STRING).build());
|
||||
|
||||
assertThat(result).isTrue();
|
||||
// The query reaches the driver verbatim — the value is never concatenated into it.
|
||||
verify(connection).prepareStatement(writeQuery);
|
||||
// The value is bound as a positional parameter, neutralising SQL injection.
|
||||
verify(preparedStatement).setString(1, maliciousValue);
|
||||
}
|
||||
|
||||
/**
|
||||
* Minimal concrete subclass so the abstract JDBC driver can be exercised in isolation.
|
||||
*/
|
||||
private static class TestJdbcDriver extends AbstractJdbcDriverCustomService {
|
||||
|
||||
TestJdbcDriver(DriverSenderService driverSenderService) {
|
||||
super(driverSenderService);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected String buildJdbcUrl(Map<String, AttributeBO> driverConfig) {
|
||||
return "jdbc:test:mem";
|
||||
}
|
||||
|
||||
@Override
|
||||
protected String getDriverClassName() {
|
||||
return "org.test.Driver";
|
||||
}
|
||||
|
||||
@Override
|
||||
protected int getDefaultPort() {
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
@@ -77,7 +77,7 @@ dc3:
|
||||
attribute-code: writeQuery
|
||||
attribute-type-flag: STRING
|
||||
default-value:
|
||||
remark: SQL UPDATE/INSERT query for writing point value
|
||||
remark: SQL UPDATE/INSERT using a single ? placeholder for the written value (bound as a parameter)
|
||||
command-attribute:
|
||||
- attribute-name: Execute Query
|
||||
attribute-code: executeQuery
|
||||
|
||||
@@ -92,7 +92,7 @@ dc3:
|
||||
attribute-code: writeQuery
|
||||
attribute-type-flag: STRING
|
||||
default-value:
|
||||
remark: SQL UPDATE/INSERT query for writing point value
|
||||
remark: SQL UPDATE/INSERT using a single ? placeholder for the written value (bound as a parameter)
|
||||
command-attribute:
|
||||
- attribute-name: Execute Query
|
||||
attribute-code: executeQuery
|
||||
|
||||
@@ -30,7 +30,7 @@ which constructs the JDBC URL `jdbc:postgresql://<host>:<port>/<database>` using
|
||||
| Attribute | Code | Type | Description |
|
||||
|-------------|------------|--------|-----------------------------------------------|
|
||||
| Read Query | readQuery | STRING | SQL SELECT query for reading point value |
|
||||
| Write Query | writeQuery | STRING | SQL UPDATE/INSERT query for writing point value |
|
||||
| Write Query | writeQuery | STRING | SQL UPDATE/INSERT using a single `?` placeholder for the value (bound as a parameter) |
|
||||
|
||||
## Command Attributes (write)
|
||||
|
||||
|
||||
@@ -77,7 +77,7 @@ dc3:
|
||||
attribute-code: writeQuery
|
||||
attribute-type-flag: STRING
|
||||
default-value:
|
||||
remark: SQL UPDATE/INSERT query for writing point value
|
||||
remark: SQL UPDATE/INSERT using a single ? placeholder for the written value (bound as a parameter)
|
||||
command-attribute:
|
||||
- attribute-name: Execute Query
|
||||
attribute-code: executeQuery
|
||||
|
||||
@@ -34,7 +34,7 @@ using the `com.microsoft.sqlserver.jdbc.SQLServerDriver`.
|
||||
| Attribute | Code | Type | Description |
|
||||
|-------------|------------|--------|-----------------------------------------------|
|
||||
| Read Query | readQuery | STRING | SQL SELECT query for reading point value |
|
||||
| Write Query | writeQuery | STRING | SQL UPDATE/INSERT query for writing point value |
|
||||
| Write Query | writeQuery | STRING | SQL UPDATE/INSERT using a single `?` placeholder for the value (bound as a parameter) |
|
||||
|
||||
## Command Attributes (write)
|
||||
|
||||
|
||||
@@ -87,7 +87,7 @@ dc3:
|
||||
attribute-code: writeQuery
|
||||
attribute-type-flag: STRING
|
||||
default-value:
|
||||
remark: SQL UPDATE/INSERT query for writing point value
|
||||
remark: SQL UPDATE/INSERT using a single ? placeholder for the written value (bound as a parameter)
|
||||
command-attribute:
|
||||
- attribute-name: Execute Query
|
||||
attribute-code: executeQuery
|
||||
|
||||
Reference in New Issue
Block a user