From 21b8e6947cc39022d7dcc71fe1840834b526055f Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Fri, 8 Aug 2014 12:48:29 +0200 Subject: [PATCH] Non authenticated request cannot use pg_ catalogs/functions --- NEWS.md | 5 +- app/controllers/app.js | 15 +++- npm-shrinkwrap.json | 2 +- package.json | 2 +- test/acceptance/app.test.js | 138 ++++++++++++++++-------------------- 5 files changed, 80 insertions(+), 82 deletions(-) diff --git a/NEWS.md b/NEWS.md index 85fa1000..8ca3e82b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,9 @@ -1.15.0 - 2014-mm-dd +1.14.1 - 2014-mm-dd ------------------- +Other changes: + + * Constraint for pg_ queries if request is non authenticated 1.14.0 - 2014-08-07 ------------------- diff --git a/app/controllers/app.js b/app/controllers/app.js index 9a43c204..7cd584c5 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -276,7 +276,7 @@ function handleQuery(req, res) { pass: global.settings.db_pubuser_pass }; - var authenticated; + var authenticated = false; var formatter; @@ -325,6 +325,7 @@ function handleQuery(req, res) { throw err; } if (_.isBoolean(isAuthenticated) && isAuthenticated) { + authenticated = isAuthenticated; dbopts.user = _.template(global.settings.db_user, {user_id: dbParams.dbuser}); if ( global.settings.hasOwnProperty('db_user_pass') ) { dbopts.pass = _.template(global.settings.db_user_pass, { @@ -386,6 +387,18 @@ function handleQuery(req, res) { tableCache.set(sql_md5, tableCacheItem); } + if ( !authenticated && tableCacheItem ) { + var affected_tables = tableCacheItem.affected_tables; + for ( var i = 0; i < affected_tables.length; ++i ) { + var t = affected_tables[i]; + if ( t.match(/\bpg_/) ) { + var e = new SyntaxError("system tables are forbidden"); + e.http_status = 403; + throw(e); + } + } + } + var fClass = formats[format]; formatter = new fClass(); req.formatter = formatter; diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 250cdb07..04382b94 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "cartodb_sql_api", - "version": "1.15.0", + "version": "1.14.1", "dependencies": { "underscore": { "version": "1.3.3" diff --git a/package.json b/package.json index a90e9272..3037836e 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "keywords": [ "cartodb" ], - "version": "1.15.0", + "version": "1.14.1", "repository": { "type": "git", "url": "git://github.com/CartoDB/CartoDB-SQL-API.git" diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 3f6a6915..ab05847d 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -955,84 +955,66 @@ test('GET /api/v1/sql ensure cross domain set on errors', function(done){ }); }); -test('cannot GET system tables', function(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, 200); - 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, 200); - req.url = pre + querystring.stringify({q: 'SELECT * FROM "pg_attribute"'}); - assert.response(app, req, function(res) { next(null, res); }); - }, - function chkSysTable3_trySysTable4(err, res) { - if ( err ) throw err; - var next = this; - assert.equal(res.statusCode, 200); - req.url = pre + querystring.stringify({q: 'SELECT a.* FROM untitle_table_4 a,pg_attribute'}); - assert.response(app, req, function(res) { next(null, res); }); - }, - function chkSysTable4_tryValidPg1(err, res) { - if ( err ) throw err; - var next = this; - assert.equal(res.statusCode, 200); - req.url = pre + querystring.stringify({q: "SELECT 'pg_'"}); - assert.response(app, req, function(res) { next(null, res); }); - }, - function chkValidPg1_tryValidPg2(err, res) { - if ( err ) throw err; - var next = this; - assert.equal(res.statusCode, 200); - req.url = pre + querystring.stringify({q: "SELECT pg_attribute FROM ( select 1 as pg_attribute ) as f"}); - assert.response(app, req, function(res) { next(null, res); }); - }, - // See http://github.com/CartoDB/CartoDB-SQL-API/issues/118 - function chkValidPg1_tryValidPg3_b(err, res) { - if ( err ) throw err; - var next = this; - assert.equal(res.statusCode, 200, res.statusCode + ':' + res.body); - req.url = pre + querystring.stringify({q: "SELECT * FROM cpg_test"}); - assert.response(app, req, function(res) { next(null, res); }); - }, - function chkValidPg2_trySet1(err, res) { - if ( err ) throw err; - var next = this; - assert.equal(res.statusCode, 200, res.statusCode + ':' + res.body); - 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); - 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); - return true; - }, - function finish(err) { - done(err); - } - ); -}); +var systemQueriesSuitesToTest = [ + { + desc: 'pg_ queries work with api_key and fail otherwise', + queries: [ + 'SELECT * FROM pg_attribute', + 'SELECT * FROM PG_attribute', + 'SELECT * FROM "pg_attribute"', + 'SELECT a.* FROM untitle_table_4 a,pg_attribute' + ], + api_key_works: true, + no_api_key_works: false + }, + { + desc: 'Possible false positive queries will work with api_key and without it', + queries: [ + "SELECT 'pg_'", + 'SELECT pg_attribute FROM ( select 1 as pg_attribute ) as f', + 'SELECT * FROM cpg_test' + ], + api_key_works: true, + no_api_key_works: true + }, + { + desc: 'Set queries will FAIL for both api_key and no_api_key queries', + queries: [ + ' SET work_mem TO 80000', + ' set statement_timeout TO 400' + ], + api_key_works: false, + no_api_key_works: false + } +]; + + systemQueriesSuitesToTest.forEach(function(suiteToTest) { + var apiKeyStatusErrorCode = !!suiteToTest.api_key_works ? 200 : 403; + testSystemQueries(suiteToTest.desc + ' with api_key', suiteToTest.queries, apiKeyStatusErrorCode, '1234'); + var noApiKeyStatusErrorCode = !!suiteToTest.no_api_key_works ? 200 : 403; + testSystemQueries(suiteToTest.desc, suiteToTest.queries, noApiKeyStatusErrorCode); + }); + + +function testSystemQueries(description, queries, statusErrorCode, apiKey) { + queries.forEach(function(query) { + test('[' + description + '] query: ' + query, function(done) { + var queryStringParams = {q: query}; + if (!!apiKey) { + queryStringParams.api_key = apiKey; + } + var request = { + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET', + url: '/api/v1/sql?' + querystring.stringify(queryStringParams) + }; + assert.response(app, request, function(response) { + assert.equal(response.statusCode, statusErrorCode); + done(); + }); + }); + }); +} test('GET decent error if domain is incorrect', function(done){ assert.response(app, {