FOLIO-3645: schema name SQL injection

pom.xml

@@ -139,6 +139,12 @@
           </configuration>
         </plugin>
 
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-surefire-plugin</artifactId>
+          <version>3.0.0-M7</version>
+        </plugin>
+
       </plugins>
     </pluginManagement>
 

tenant/src/main/java/org/folio/spring/tenant/hibernate/HibernateSchemaService.java

@@ -6,6 +6,7 @@
 import java.nio.charset.StandardCharsets;
 import java.sql.Connection;
 import java.sql.DriverManager;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
@@ -41,8 +42,6 @@
 import org.springframework.core.type.filter.AnnotationTypeFilter;
 import org.springframework.stereotype.Service;
 
-import static org.hibernate.cfg.AvailableSettings.*;
-
 @Service("hibernateSchemaService")
 public class HibernateSchemaService implements InitializingBean {
 
@@ -89,7 +88,7 @@ public void afterPropertiesSet() throws Exception {
       String tenant = tenantProperties.getDefaultTenant();
       Map<String, String> settings = getSettings(tenant);
       try (Connection connection = getConnection(settings)) {
-        if (!schemaExists(connection, tenant)) {
+        if (!schemaExists(connection, settings)) {
           initializeSchema(connection, settings);
         }
       }
@@ -99,7 +98,7 @@ public void afterPropertiesSet() throws Exception {
   public void createTenant(String tenant) throws SQLException, IOException {
     Map<String, String> settings = getSettings(tenant);
     try (Connection connection = getConnection(settings)) {
-      if (schemaExists(connection, tenant)) {
+      if (schemaExists(connection, settings)) {
         throw new TenantAlreadyExistsException("Tenant already exists: " + tenant);
       }
       initializeSchema(connection, settings);
@@ -109,7 +108,7 @@ public void createTenant(String tenant) throws SQLException, IOException {
   public void deleteTenant(String tenant) throws SQLException {
     Map<String, String> settings = getSettings(tenant);
     try (Connection connection = getConnection(settings)) {
-      if (!schemaExists(connection, tenant)) {
+      if (!schemaExists(connection, settings)) {
         throw new TenantDoesNotExistsException("Tenant does not exist: " + tenant);
       }
       dropSchema(connection, getSchema(settings));
@@ -119,7 +118,7 @@ public void deleteTenant(String tenant) throws SQLException {
   public boolean schemaExists(String tenant) throws SQLException {
     Map<String, String> settings = getSettings(tenant);
     try (Connection connection = getConnection(settings)) {
-      return schemaExists(connection, getSchema(settings));
+      return schemaExists(connection, settings);
     }
   }
 
@@ -176,10 +175,11 @@ private void dropSchema(Connection connection, String schema) throws SQLExceptio
     }
   }
 
-  private boolean schemaExists(Connection connection, String schema) throws SQLException {
-    try (Statement statement = connection.createStatement()) {
-      String queryTemplate = "SELECT EXISTS(SELECT 1 FROM information_schema.schemata WHERE schema_name = '%s');";
-      ResultSet resultSet = statement.executeQuery(String.format(queryTemplate, schema));
+  private boolean schemaExists(Connection connection, Map<String, String> settings) throws SQLException {
+    String sql = "SELECT EXISTS(SELECT 1 FROM information_schema.schemata WHERE schema_name = ?)";
+    try (PreparedStatement statement = connection.prepareStatement(sql)) {
+      statement.setString(1, getSchema(settings));
+      ResultSet resultSet = statement.executeQuery();
       if (resultSet.next()) {
         return resultSet.getBoolean(1);
       }

tenant/src/main/java/org/folio/spring/tenant/service/SchemaService.java

@@ -1,17 +1,27 @@
 package org.folio.spring.tenant.service;
 
+import java.util.Locale;
+import java.util.regex.Pattern;
 import org.folio.spring.tenant.properties.BuildInfoProperties;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 
 @Service
 public class SchemaService {
 
+  // allow list of characters prevents SQL injection
+  private static final Pattern SCHEMA_REGEXP = Pattern.compile("[a-z0-9_]+");
+
   @Autowired
   private BuildInfoProperties buildInfoProperties;
 
   public String getSchema(String tenant) {
-    return String.format("%s_%s", tenant, buildInfoProperties.getArtifact()).replace("-", "_");
+    String schema = String.format("%s_%s", tenant, buildInfoProperties.getArtifact())
+        .replace("-", "_").toLowerCase(Locale.ROOT);
+    if (! SCHEMA_REGEXP.matcher(schema).matches()) {
+      throw new IllegalArgumentException("Illegal character in schema name: " + schema);
+    }
+    return schema;
   }
 
 }

tenant/src/test/java/org/folio/spring/tenant/TestApplication.java

@@ -0,0 +1,20 @@
+package org.folio.spring.tenant;
+
+import org.springframework.boot.SpringApplication;
+import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.boot.builder.SpringApplicationBuilder;
+import org.springframework.boot.web.servlet.support.SpringBootServletInitializer;
+
+@SpringBootApplication(scanBasePackages = "org.folio.spring")
+public class TestApplication extends SpringBootServletInitializer {
+
+  @Override
+  protected SpringApplicationBuilder configure(SpringApplicationBuilder application) {
+    return application.sources(TestApplication.class);
+  }
+
+  public static void main(String[] args) {
+    SpringApplication.run(TestApplication.class, args);
+  }
+
+}

tenant/src/test/java/org/folio/spring/tenant/controller/TenantControllerTest.java

@@ -0,0 +1,31 @@
+package org.folio.spring.tenant.controller;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import org.folio.spring.tenant.exception.TenantAlreadyExistsException;
+import org.folio.spring.tenant.exception.TenantDoesNotExistsException;
+import org.folio.spring.tenant.model.request.TenantAttributes;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+
+@SpringBootTest
+class TenantControllerTest {
+
+  @Autowired
+  TenantController tenantController;
+
+  TenantAttributes tenantAttributes = new TenantAttributes("1.0.0");
+
+  @Test
+  void createAndDelete() throws Exception {
+    tenantController.create("a", tenantAttributes);
+    assertThrows(TenantAlreadyExistsException.class, () -> tenantController.create("a", tenantAttributes));
+    tenantController.create("b", tenantAttributes);
+    tenantController.delete("b");
+    assertThrows(TenantDoesNotExistsException.class, () -> tenantController.delete("b"));
+    assertThrows(TenantDoesNotExistsException.class, () -> tenantController.delete("c"));
+    tenantController.delete("a");
+  }
+
+}

tenant/src/test/java/org/folio/spring/tenant/service/SchemaServiceTest.java

@@ -0,0 +1,34 @@
+package org.folio.spring.tenant.service;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import org.folio.spring.tenant.properties.BuildInfoProperties;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.context.properties.EnableConfigurationProperties;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.TestPropertySource;
+
+@SpringBootTest(classes = SchemaService.class)
+@EnableConfigurationProperties(value = BuildInfoProperties.class)
+@TestPropertySource(properties = "info.build.artifact=mod-Baz")
+class SchemaServiceTest {
+
+  @Autowired
+  private SchemaService schemaService;
+
+  @Test
+  void validSchema() {
+    assertThat(schemaService.getSchema("FooBar123"), is("foobar123_mod_baz"));
+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = { "ä", "?", "abc+xyz", "a1." })
+  void invalidSchema(String tenant) {
+    assertThrows(IllegalArgumentException.class, () -> schemaService.getSchema(tenant));
+  }
+}

tenant/src/test/resources/application.yml

@@ -0,0 +1,53 @@
+info.build.artifact: mod-example
+
+logging:
+  file: logs/mod-workflow-tests.log
+  level:
+    org:
+      folio: INFO
+      hibernate: INFO
+      springframework: INFO
+  path:
+
+server:
+  port: 9101
+
+spring:
+  application.name: mod-example
+  data.rest:
+    returnBodyOnCreate: true
+    returnBodyOnUpdate: true
+  datasource:
+    platform: h2
+    url: jdbc:h2:mem:AZ;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE;MODE=PostgreSQL;DATABASE_TO_LOWER=true;DEFAULT_NULL_ORDERING=HIGH
+    driverClassName: org.h2.Driver
+    username: folio
+    password: folio
+  h2:
+    console:
+      enabled: true
+      path: /h2console
+  jpa:
+    database-platform: org.hibernate.dialect.H2Dialect
+    properties.hibernate.jdbc.lob.non_contextual_creation: true
+    generate-ddl: false
+    hibernate.ddl-auto: none
+    open-in-view: true
+    show-sql: false
+  profiles:
+    active: default
+  thymeleaf:
+    mode: TEXT
+    suffix: .sql
+
+event.queue.name: event.queue
+
+tenant:
+  header-name: X-Okapi-Tenant
+  force-tenant: false
+  default-tenant: diku
+  initialize-default-tenant: true
+  domain-packages:
+  schema-scripts:
+
+okapi.url: http://localhost:9130