Sanitize the string to avoid a connection string injection

backend/dataall/db/connection.py

@@ -29,19 +29,19 @@ def __init__(self, dbconfig: DbConfig):
             dbconfig.url,
             echo=False,
             pool_size=1,
-            connect_args={'options': f"-csearch_path={dbconfig.params['schema']}"},
+            connect_args={'options': f"-csearch_path={dbconfig.schema}"},
         )
         try:
             if not self.engine.dialect.has_schema(
-                self.engine, dbconfig.params['schema']
+                self.engine, dbconfig.schema
             ):
                 log.info(
-                    f"Schema not found - init the schema {dbconfig.params['schema']}"
+                    f"Schema not found - init the schema {dbconfig.schema}"
                 )
                 self.engine.execute(
-                    sqlalchemy.schema.CreateSchema(dbconfig.params['schema'])
+                    sqlalchemy.schema.CreateSchema(dbconfig.schema)
                 )
-            log.info('-- Using schema: %s --', dbconfig.params['schema'])
+            log.info('-- Using schema: %s --', dbconfig.schema)
         except Exception as e:
             log.error(f'Could not create schema: {e}')
 
@@ -124,10 +124,12 @@ def get_engine(envname=ENVNAME):
         creds = json.loads(db_credentials_string['SecretString'])
         user = creds['username']
         pwd = creds['password']
+        host = param_store.get_parameter(env=envname, path='aurora/hostname')
+        database = param_store.get_parameter(env=envname, path='aurora/db')
+
         db_params = {
-            'host': param_store.get_parameter(env=envname, path='aurora/hostname'),
-            'port': param_store.get_parameter(env=envname, path='aurora/port'),
-            'db': param_store.get_parameter(env=envname, path='aurora/db'),
+            'host': host,
+            'db': database,
             'user': user,
             'pwd': pwd,
             'schema': schema,
@@ -136,7 +138,6 @@ def get_engine(envname=ENVNAME):
         hostname = 'db' if envname == 'dkrcompose' else 'localhost'
         db_params = {
             'host': hostname,
-            'port': '5432',
             'db': 'dataall',
             'user': 'postgres',
             'pwd': 'docker',

backend/dataall/db/dbconfig.py

@@ -1,23 +1,55 @@
+import os
+import re
+
+_SANITIZE_WORD_REGEX = r"[^\w]"  # A-Za-z0-9_
+_SANITIZE_HOST_REGEX = r"[^\w.-]"
+_SANITIZE_PWD_REGEX = r"[\"\s%+~`#$&*()|\[\]{}:;<>?!'/]+"
+_AURORA_HOST_SUFFIX = "rds.amazonaws.com"
+_POSTGRES_MAX_LEN = 63
+_MAX_HOST_LENGTH = 253
+
+_envname = os.getenv('envname', 'local')
+
+
 class DbConfig:
-    def __init__(self, **kwargs):
-        self.params = kwargs
-        self.url = f"postgresql+pygresql://{self.params['user']}:{self.params['pwd']}@{self.params['host']}/{self.params['db']}"
+    def __init__(self, user: str, pwd: str, host: str, db: str, schema: str):
+        for param in (user, db, schema):
+            if len(param) > _POSTGRES_MAX_LEN:
+                raise ValueError(
+                    f"PostgreSQL doesn't allow values more than 63 characters"
+                    f" parameters {user}, {db}, {schema}"
+                )
+
+        if len(host) > _MAX_HOST_LENGTH:
+            raise ValueError(f"Hostname is too long: {host}")
+
+        if _envname not in ['local', 'pytest', 'dkrcompose'] and not host.lower().endswith(_AURORA_HOST_SUFFIX):
+            raise ValueError(f"Unknown host {host} for the rds")
+
+        self.user = self._sanitize(_SANITIZE_WORD_REGEX, user)
+        self.host = self._sanitize(_SANITIZE_HOST_REGEX, host)
+        self.db = self._sanitize(_SANITIZE_WORD_REGEX, db)
+        self.schema = self._sanitize(_SANITIZE_WORD_REGEX, schema)
+        pwd = self._sanitize(_SANITIZE_PWD_REGEX, pwd)
+        self.url = f"postgresql+pygresql://{self.user}:{pwd}@{self.host}/{self.db}"
 
     def __str__(self):
-        lines = []
-        lines.append('  DbConfig >')
+        lines = ['  DbConfig >']
         hr = ' '.join(['+', ''.ljust(10, '-'), '+', ''.ljust(65, '-'), '+'])
         lines.append(hr)
         header = ' '.join(['+', 'Db Param'.ljust(10), ' ', 'Value'.ljust(65), '+'])
         lines.append(header)
         hr = ' '.join(['+', ''.ljust(10, '-'), '+', ''.ljust(65, '-'), '+'])
         lines.append(hr)
-        for k in self.params:
-            v = self.params[k]
-            if k == 'pwd':
-                v = '*' * len(self.params[k])
-            lines.append(' '.join(['|', k.ljust(10), '|', v.ljust(65), '|']))
+        lines.append(' '.join(['|', "host".ljust(10), '|', self.host.ljust(65), '|']))
+        lines.append(' '.join(['|', "db".ljust(10), '|', self.db.ljust(65), '|']))
+        lines.append(' '.join(['|', "user".ljust(10), '|', self.user.ljust(65), '|']))
+        lines.append(' '.join(['|', "pwd".ljust(10), '|', "*****".ljust(65), '|']))
 
         hr = ' '.join(['+', ''.ljust(10, '-'), '+', ''.ljust(65, '-'), '+'])
         lines.append(hr)
         return '\n'.join(lines)
+
+    @staticmethod
+    def _sanitize(regex, string: str) -> str:
+        return re.sub(regex, "", string)

deploy/stacks/aurora.py

@@ -29,6 +29,7 @@ def __init__(
     ):
         super().__init__(scope, id, **kwargs)
 
+        # if exclude_characters property is set make sure that the pwd regex in DbConfig is changed accordingly
         db_credentials = rds.DatabaseSecret(
             self, f'{resource_prefix}-{envname}-aurora-db', username='dtaadmin'
         )
@@ -152,12 +153,6 @@ def __init__(
             parameter_name=f'/dataall/{envname}/aurora/hostname',
             string_value=str(database.cluster_endpoint.hostname),
         )
-        ssm.StringParameter(
-            self,
-            'DatabasePortParameter',
-            parameter_name=f'/dataall/{envname}/aurora/port',
-            string_value=str(database.cluster_endpoint.port),
-        )
 
         ssm.StringParameter(
             self,

tests/db/test_dbconfig.py

@@ -0,0 +1,57 @@
+from dataall.db import DbConfig
+
+
+def test_sanitize_database():
+    config = DbConfig(
+        user='dataall',
+        pwd='123456789',
+        host="dataall.eu-west-1.rds.amazonaws.com",
+        db='dataall\'; DROP TABLE users;',
+        schema='dev'
+    )
+
+    # connection injection: 'postgresql+pygresql://dataall:123456789@dataall.eu-west-1.rds.amazonaws.com/dataall'; DROP TABLE users;
+    assert config.url == \
+           'postgresql+pygresql://dataall:123456789@dataall.eu-west-1.rds.amazonaws.com/dataallDROPTABLEusers'
+
+
+def test_sanitize_user():
+    config = DbConfig(
+        user='dataall2;^&*end',
+        pwd='qwsufn3i20d-_s3qaSW3d2',
+        host="dataall.eu-west-1.rds.amazonaws.com",
+        db='dataall',
+        schema='dev'
+    )
+
+    assert config.url == \
+           'postgresql+pygresql://dataall2end:qwsufn3i20d-_s3qaSW3d2@dataall.eu-west-1.rds.amazonaws.com/dataall'
+
+
+def test_sanitize_pwd():
+    config = DbConfig(
+        user='dataall',
+        pwd='qazxsVFRTGBdfrew-332_c2@dataall.eu-west-1.rds.amazonaws.com/dataall\'; drop table dataset; # ',
+        host="dataall.eu-west-1.rds.amazonaws.com",
+        db='dataall',
+        schema='dev'
+    )
+
+    # without sanitation should be :
+    # postgresql+pygresql://dataall:qazxsVFRTGBdfrew-332_c2@dataall.eu-west-1.rds.amazonaws.com/dataall'
+    # drop table dataset; # @dataall.eu-west-1.rds.amazonaws.com/dataall
+    assert config.url == \
+           "postgresql+pygresql://dataall:qazxsVFRTGBdfrew-332_c2@dataall.eu-west-1.rds.amazonaws.com" \
+           "dataalldroptabledataset@dataall.eu-west-1.rds.amazonaws.com/dataall"
+
+
+def test_sanitize_host():
+    config = DbConfig(
+        user='dataall',
+        pwd='q68rjdmwiosoxahGDYJWIdi-9eu93_9dJJ_',
+        host="dataall.eu-west-1$%#&@*#)$#.rds.amazonaws.com",
+        db='dataall',
+        schema='dev'
+    )
+
+    assert config.url == "postgresql+pygresql://dataall:q68rjdmwiosoxahGDYJWIdi-9eu93_9dJJ_@dataall.eu-west-1.rds.amazonaws.com/dataall"