Merge lp:~openerp-dev/openobject-server/7.0-sanitize-db-connections into lp:openobject-server/7.0

[Olivier Dony (Odoo)]

Proof of concept of database names sanitization at the connection pool level. This would have the side effect of preventing trivial exploits of PostgreSQL's CVE-2013-1899 vulnerability on unpatched systems.

This works by enforcing an arbitrary pattern for allowed database names: ^[\w][\w.-]+

This is unlikely to be merged in 7.0 as it would constitute a backwards-incompatible change on an LTS version: connecting to databases that do not match this pattern would be impossible, even though the database may be legit and working. There is no easy way to detect exploit attempts accurately because the database names used for the exploit are technically valid database names that ought to be supported by PostgreSQL.

We could consider merging this in trunk, but it would be much less useful to prevent the exploit. If we do that, we should also take into account the performance hit of the extra regex check (the regex check is performed *very often* - usually more than once per incoming request). DSNs should probably be cached - they really only need to be computed once.

Unmerged revisions

4976. By Olivier Dony (Odoo) on 2013-05-16

[FIX] sql_db: sanitize database names before connecting to them - prevents exploiting PostgreSQL CVE-2013-1899 vulnerability

The Database Manager screen enforces this pattern: ^[a-zA-Z][a-zA-Z0-9_-]+$
so we are just a bit less restrictive and allow leading numbers and
underscores for manually created databases.

=== modified file 'openerp/sql_db.py'
--- openerp/sql_db.py	2013-03-01 12:07:44 +0000
+++ openerp/sql_db.py	2013-05-16 14:06:50 +0000
@@ -496,13 +496,21 @@
         except Exception:
             return False
 
+# DB name sanitization, a.o. to prevent exploits of PostgreSQL
+# security vulnerability CVE-2013-1899
+# The Database Manager screen enforces this pattern: ^[a-zA-Z][a-zA-Z0-9_-]+$
+# so we're just a bit less restrictive and allow leading numbers and
+# underscores for manually created databases.
+VALID_DB_NAMES_EXPRESSION = re.compile(r'^[\w][\w.-]+')
+
 def dsn(db_name):
     _dsn = ''
     for p in ('host', 'port', 'user', 'password'):
         cfg = tools.config['db_' + p]
         if cfg:
             _dsn += '%s=%s ' % (p, cfg)
-
+    if not VALID_DB_NAMES_EXPRESSION.match(db_name):
+        raise ValueError('Unsupported database name: %r' % db_name)
     return '%sdbname=%s' % (_dsn, db_name)
 
 def dsn_are_equals(first, second):