From 1bcffbc68c2a437f708b5e7828c48376bf6d4bf0 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@keybit.net>
Date: Tue, 9 Apr 2013 11:49:05 +0200
Subject: [PATCH] Make using SET or querying system catalogues harder

An hack to "prevent" querying system tables already existed but
was pretty weak. This commits makes that a bit stronger. The
filter for SET is new.
---
 NEWS.md                     |  1 +
 app/models/psql.js          | 18 ++++++---
 test/acceptance/app.test.js | 75 +++++++++++++++++++++++++++++++------
 3 files changed, 77 insertions(+), 17 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 1adfccc8..fdfa3a05 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,6 @@
 1.3.8
 -----
+* Make using SET or querying system catalogues harder
 
 1.3.7
 -----
diff --git a/app/models/psql.js b/app/models/psql.js
index b0874742..cef2dc0b 100644
--- a/app/models/psql.js
+++ b/app/models/psql.js
@@ -99,15 +99,23 @@ var PSQL = function(user_id, db, limit, offset){
         }
     };
 
-    // throw exception if system table detected
+    // throw exception if illegal operations are detected
+    // NOTE: this check is weak hack, better database
+    //       permissions should be used instead.
     me.sanitize = function(sql, callback){
-        if (sql.match(/\s+pg_.+/)){
+        if (sql.match(/\s+"?pg_.+/i)){
             var error = new SyntaxError("system tables are forbidden");
             error.http_status = 403;
-            throw error;
-        } else {
-            callback(null,true);
+            callback(error); 
+            return;
         }
+        if (sql.match(/^\s+set\s+/i)){
+            var error = new SyntaxError("SET command is forbidden");
+            error.http_status = 403;
+            callback(error); 
+            return;
+        }
+        callback(null,true);
     };
 
     return me;
diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js
index b22b6e48..6a92a0d9 100644
--- a/test/acceptance/app.test.js
+++ b/test/acceptance/app.test.js
@@ -22,6 +22,7 @@ var app    = require(global.settings.app_root + '/app/controllers/app')
     , zipfile = require('zipfile')
     , fs      = require('fs')
     , libxmljs = require('libxmljs')
+    , Step = require('step')
     ;
 
 // allow lots of emitters to be set to silence warning
@@ -621,18 +622,68 @@ test('GET /api/v1/sql ensure cross domain set on errors', function(done){
 });
 
 test('cannot GET system tables', function(done){
-    assert.response(app, {
-        url: '/api/v1/sql?q=SELECT%20*%20FROM%20pg_attribute',
-        headers: {host: 'vizzuality.cartodb.com'},
-        method: 'GET'
-    },{
-        status: 403
-    }, function(res) {
-      assert.deepEqual(res.headers['content-type'], 'application/json; charset=utf-8');
-      assert.deepEqual(res.headers['content-disposition'], 'inline');
-      // TODO: check actual error message...
-      done();
-    });
+    var req = { headers: {host: 'vizzuality.cartodb.com'},
+        method: 'GET' };
+    var pre = '/api/v1/sql?';
+    Step(
+      function trySysTable1() {
+        req.url = pre + querystring.stringify({q: 'SELECT * FROM pg_attribute'});
+        var next = this;
+        assert.response( app, req, function(res) { next(null, res); } );
+      },
+      function chkSysTable1_trySysTable2(err, res) {
+        if ( err ) throw err;
+        var next = this;
+        assert.equal(res.statusCode, 403);
+        assert.deepEqual(res.headers['content-type'], 'application/json; charset=utf-8');
+        assert.deepEqual(res.headers['content-disposition'], 'inline');
+        // TODO: check actual error message...
+        req.url = pre + querystring.stringify({q: 'SELECT * FROM PG_attribute'});
+        assert.response(app, req, function(res) { next(null, res); });
+      },
+      function chkSysTable2_trySysTable3(err, res) {
+        if ( err ) throw err;
+        var next = this;
+        assert.equal(res.statusCode, 403);
+        assert.deepEqual(res.headers['content-type'], 'application/json; charset=utf-8');
+        assert.deepEqual(res.headers['content-disposition'], 'inline');
+        // TODO: check actual error message...
+        req.url = pre + querystring.stringify({q: 'SELECT * FROM "pg_attribute"'});
+        assert.response(app, req, function(res) { next(null, res); });
+      },
+      function chkSysTable3_trySet1(err, res) {
+        if ( err ) throw err;
+        var next = this;
+        assert.equal(res.statusCode, 403);
+        assert.deepEqual(res.headers['content-type'], 'application/json; charset=utf-8');
+        assert.deepEqual(res.headers['content-disposition'], 'inline');
+        // TODO: check actual error message...
+        req.url = pre + querystring.stringify({q: ' set statement_timeout TO 400'});
+        assert.response(app, req, function(res) { next(null, res); });
+      },
+      function chkSet1_trySet2(err, res) {
+        if ( err ) throw err;
+        var next = this;
+        assert.equal(res.statusCode, 403);
+        assert.deepEqual(res.headers['content-type'], 'application/json; charset=utf-8');
+        assert.deepEqual(res.headers['content-disposition'], 'inline');
+        // TODO: check actual error message...
+        req.url = pre + querystring.stringify({q: ' SET work_mem TO 80000'});
+        assert.response(app, req, function(res) { next(null, res); });
+      },
+      function chkSet2(err, res) {
+        if ( err ) throw err;
+        var next = this;
+        assert.equal(res.statusCode, 403);
+        assert.deepEqual(res.headers['content-type'], 'application/json; charset=utf-8');
+        assert.deepEqual(res.headers['content-disposition'], 'inline');
+        // TODO: check actual error message...
+        return true;
+      },
+      function finish(err) {
+        done(err);
+      }
+    );
 });
 
 test('GET decent error if domain is incorrect', function(done){