postgres.db_create: fix handling of empty string (#33261)

* modules.postgres: fix handling of empty string in db_create

The code responsible for collecting the parameters used in the CREATE
DATABASE query works unexpectedly in case for values which are evaluated
to False as bool, but are not None.

This caused queries with missing rvalues like this one (lc_collate=""):
2016-05-13 16:04:05,243 [salt.loaded.int.module.cmdmod][ERROR   ][2990]
stderr: ERROR:  syntax error at or near "OWNER"
LINE 1: ..._production" WITH ENCODING = 'utf8' LC_COLLATE =  OWNER =
"g...

Please note that proper escaping or a different approach would be needed
here.

* modules.postgres: handle trivial sqli in db_create

* modules.postgres: fix OrderedDict usage

* modules.postgres: quote TABLESPACE too in db_create

salt/modules/postgres.py

@@ -435,6 +435,16 @@ def db_exists(name, user=None, host=None, port=None, maintenance_db=None,
     return name in databases
 
 
+# TODO properly implemented escaping
+def _quote_ddl_value(value, quote="'"):
+    if value is None:
+        return None
+    if quote in value:  # detect trivial sqli
+        raise SaltInvocationError(
+            'Unsupported character {0} in value: {1}'.format(quote, value))
+    return "{quote}{value}{quote}".format(quote=quote, value=value)
+
+
 def db_create(name,
               user=None,
               host=None,
@@ -465,16 +475,16 @@ def db_create(name,
     query = 'CREATE DATABASE "{0}"'.format(name)
 
     # "With"-options to create a database
-    with_args = salt.utils.odict.OrderedDict({
+    with_args = salt.utils.odict.OrderedDict([
+        ('TABLESPACE', _quote_ddl_value(tablespace, '"')),
         # owner needs to be enclosed in double quotes so postgres
         # doesn't get thrown by dashes in the name
-        'OWNER': owner and '"{0}"'.format(owner),
-        'TEMPLATE': template,
-        'ENCODING': encoding and '\'{0}\''.format(encoding),
-        'LC_COLLATE': lc_collate and '\'{0}\''.format(lc_collate),
-        'LC_CTYPE': lc_ctype and '\'{0}\''.format(lc_ctype),
-        'TABLESPACE': tablespace,
-    })
+        ('OWNER', _quote_ddl_value(owner, '"')),
+        ('TEMPLATE', template),
+        ('ENCODING', _quote_ddl_value(encoding)),
+        ('LC_COLLATE', _quote_ddl_value(lc_collate)),
+        ('LC_CTYPE', _quote_ddl_value(lc_ctype)),
+    ])
     with_chunks = []
     for key, value in with_args.items():
         if value is not None:

tests/unit/modules/postgres_test.py

@@ -13,6 +13,7 @@
 
 # Import salt libs
 from salt.modules import postgres
+from salt.exceptions import SaltInvocationError
 
 postgres.__grains__ = None  # in order to stub it w/patch below
 postgres.__salt__ = None  # in order to stub it w/patch below
@@ -144,11 +145,34 @@ def test_db_create(self):
             ['/usr/bin/pgsql', '--no-align', '--no-readline',
              '--no-password', '--username', 'testuser', '--host',
              'testhost', '--port', 'testport', '--dbname', 'maint_db',
-             '-c', 'CREATE DATABASE "dbname" WITH TABLESPACE = testspace '
+             '-c', 'CREATE DATABASE "dbname" WITH TABLESPACE = "testspace" '
                    'OWNER = "otheruser"'],
             host='testhost', user='testuser',
             password='foo', runas='foo', port='testport')
 
+    @patch('salt.modules.postgres._run_psql',
+           Mock(return_value={'retcode': 0}))
+    def test_db_create_empty_string_param(self):
+        postgres.db_create('dbname', lc_collate='', encoding='utf8',
+                user='testuser', host='testhost', port=1234,
+                maintenance_db='maint_db', password='foo')
+
+        postgres._run_psql.assert_called_once_with(
+                ['/usr/bin/pgsql', '--no-align', '--no-readline',
+                    '--no-password', '--username', 'testuser', '--host',
+                    'testhost', '--port', '1234', '--dbname', 'maint_db', '-c',
+                    'CREATE DATABASE "dbname" WITH ENCODING = \'utf8\' '
+                    'LC_COLLATE = \'\''], host='testhost', password='foo',
+                port=1234, runas=None, user='testuser')
+
+    @patch('salt.modules.postgres._run_psql',
+           Mock(return_value={'retcode': 0}))
+    def test_db_create_with_trivial_sql_injection(self):
+        self.assertRaises(
+                SaltInvocationError,
+                postgres.db_create,
+                'dbname', lc_collate="foo' ENCODING='utf8")
+
     @patch('salt.modules.postgres._run_psql',
            Mock(return_value={'retcode': 0,
                               'stdout': test_list_db_csv}))