cartodb/CartoDB-SQL-API
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rework system catalogue prevention access check to use CDB_QueryTables

Browse Source 
This change reduces the chances of false positive
(forbidding legit queries). Doesn't solve the problem of false
negative (allowing illegit queries).
This commit is contained in:
Sandro Santilli 2013-04-09 12:36:37 +02:00
parent d54d953e75
commit e7437ba7cd
3 changed files with 28 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
app/controllers/app.js
View File

@ -208,6 +208,18 @@ function handleQuery(req, res) {
}
}
if ( tableCacheItem ) {
var affected_tables = tableCacheItem.affected_tables.split(/^\{(.*)\}$/)[1].split(',');
for ( var i=0; i<affected_tables.length; ++i ) {
var t = affected_tables[i];
if ( t.match(/\.?pg_/) ) {
var e = new SyntaxError("system tables are forbidden");
e.http_status = 403;
throw(e);
}
}
}
// TODO: refactor formats to external object
if (format === 'geojson' || format === 'topojson' ){
sql = 'SELECT *, ST_AsGeoJSON(' + gn + ',' + dp

7
app/models/psql.js
View File

@ -103,12 +103,7 @@ var PSQL = function(user_id, db, limit, offset){
// NOTE: this check is weak hack, better database
// permissions should be used instead.
me.sanitize = function(sql, callback){
if (sql.match(/\bpg_.+/i)){
var error = new SyntaxError("system tables are forbidden");
error.http_status = 403;
callback(error);
return;
}
// NOTE: illegal table access is checked in main app
if (sql.match(/^\s+set\s+/i)){
var error = new SyntaxError("SET command is forbidden");
error.http_status = 403;

35
test/acceptance/app.test.js
View File

@ -635,9 +635,6 @@ test('cannot GET system tables', function(done){
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); });
},
@ -645,9 +642,6 @@ test('cannot GET system tables', function(done){
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); });
},
@ -655,19 +649,27 @@ test('cannot GET system tables', function(done){
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 a.* FROM untitle_table_4 a,pg_attribute'});
assert.response(app, req, function(res) { next(null, res); });
},
function chkSysTable4_trySet1(err, res) {
function chkSysTable4_tryValidPg1(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 '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); });
},
function chkValidPg2_trySet1(err, res) {
if ( err ) throw err;
var next = this;
assert.equal(res.statusCode, 200);
req.url = pre + querystring.stringify({q: ' set statement_timeout TO 400'});
assert.response(app, req, function(res) { next(null, res); });
},
@ -675,19 +677,12 @@ test('cannot GET system tables', function(done){
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) {