From 025f201ea870f4f729b623eab10c8bfa36341f0d Mon Sep 17 00:00:00 2001 From: Simon Tokumine Date: Tue, 22 Nov 2011 00:06:14 +0000 Subject: [PATCH] add system table sanitizer --- app/controllers/app.js | 7 +- app/models/psql.js | 152 +++++++++++++++++++----------------- test/acceptance/app.test.js | 10 +-- 3 files changed, 93 insertions(+), 76 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 65854454..27ad51f0 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -159,7 +159,12 @@ function handleException(err, res){ console.log(err.stack); } - res.send(msg, 500); + // if the exception defines a http status code, use that, else a 500 + if (!_.isUndefined(err.http_status)){ + res.send(msg, err.http_status); + } else { + res.send(msg, 400); + } } module.exports = app; diff --git a/app/models/psql.js b/app/models/psql.js index a55cd80e..7523dd7f 100644 --- a/app/models/psql.js +++ b/app/models/psql.js @@ -1,7 +1,7 @@ var _ = require('underscore') - , Step = require('step') - , pg = require('pg');//.native; // disabled for now due to: https://github.com/brianc/node-postgres/issues/48 - _.mixin(require('underscore.string')); + , Step = require('step') + , pg = require('pg');//.native; // disabled for now due to: https://github.com/brianc/node-postgres/issues/48 +_.mixin(require('underscore.string')); // PSQL // @@ -11,85 +11,97 @@ var _ = require('underscore') // * defaults to connecting with a "READ ONLY" user to given DB if not passed a specific user_id var PSQL = function(user_id, db, limit, offset){ - var error_text = "Incorrect access parameters. If you are accessing via OAuth, please check your tokens are correct. For public users, please ensure your table is published." - if (!_.isString(user_id) && !_.isString(db)) throw new Error(error_text); + var error_text = "Incorrect access parameters. If you are accessing via OAuth, please check your tokens are correct. For public users, please ensure your table is published." + if (!_.isString(user_id) && !_.isString(db)) throw new Error(error_text); - var me = { - public_user: "publicuser" - , user_id: user_id - , db: db - , limit: limit - , offset: offset - , client: null - }; + var me = { + public_user: "publicuser" + , user_id: user_id + , db: db + , limit: limit + , offset: offset + , client: null + }; - me.username = function(){ - var username = this.public_user; - if (_.isString(this.user_id)) - username = _.template(global.settings.db_user, {user_id: this.user_id}); + me.username = function(){ + var username = this.public_user; + if (_.isString(this.user_id)) + username = _.template(global.settings.db_user, {user_id: this.user_id}); - return username; - }; + return username; + }; - me.database = function(){ - var database = db; - if (_.isString(this.user_id)) - database = _.template(global.settings.db_base_name, {user_id: this.user_id}); + me.database = function(){ + var database = db; + if (_.isString(this.user_id)) + database = _.template(global.settings.db_base_name, {user_id: this.user_id}); - return database; - }; + return database; + }; - // memoizes connection in object. move to proper pool. - me.connect = function(callback){ - var that = this - var conString = "tcp://" + this.username() + "@" + global.settings.db_host + ":" + global.settings.db_port + "/" + this.database(); + // memoizes connection in object. move to proper pool. + me.connect = function(callback){ + var that = this + var conString = "tcp://" + this.username() + "@" + global.settings.db_host + ":" + global.settings.db_port + "/" + this.database(); - if (that.client) { - return callback(null, that.client); - } else { - pg.connect(conString, function(err, client){ - that.client = client; - return callback(err, client); - }); - } - }; - - me.query = function(sql, callback){ - var that = this; - - Step( - function(){ - that.connect(this); - }, - function(err, client){ - if (err) return callback(err, null); - client.query(that.window_sql(sql), this); - }, - function(err, res){ - //if (err) console.log(err); - callback(err, res) + if (that.client) { + return callback(null, that.client); + } else { + pg.connect(conString, function(err, client){ + that.client = client; + return callback(err, client); + }); } - ); - }; + }; - me.end = function(){ - this.client.end(); - }; + me.query = function(sql, callback){ + var that = this; - // little hack for UI - me.window_sql = function(sql){ - // only window select functions - if (_.isNumber(this.limit) && _.isNumber(this.offset) && /^\s*SELECT.*$/.test(sql.toUpperCase())){ - return "SELECT * FROM (" + sql + ") AS cdbq_1 LIMIT " + this.limit + " OFFSET " + this.offset; - } else { - return sql; - } - }; + Step( + function(){ + that.sanitize(sql, this); + }, + function(err, clean){ + if (err) throw err; + that.connect(this); + }, + function(err, client){ + if (err) return callback(err, null); + client.query(that.window_sql(sql), this); + }, + function(err, res){ + //if (err) console.log(err); + callback(err, res) + } + ); + }; - // throw exception if system table detected - + me.end = function(){ + this.client.end(); + }; - return me; + // little hack for UI + me.window_sql = function(sql){ + // only window select functions + if (_.isNumber(this.limit) && _.isNumber(this.offset) && /^\s*SELECT.*$/.test(sql.toUpperCase())){ + return "SELECT * FROM (" + sql + ") AS cdbq_1 LIMIT " + this.limit + " OFFSET " + this.offset; + } else { + return sql; + } + }; + + // throw exception if system table detected + me.sanitize = function(sql, callback){ + if (sql.match(/\s+pg_.+/)){ + var error = new SyntaxError("system tables are forbidden"); + error.http_status = 403; + throw error; + } else { + callback(null,true); + } + }; + + return me; }; module.exports = PSQL; diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 670de11a..08a9d1a2 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -28,7 +28,7 @@ tests['GET /api/v1/sql'] = function(){ method: 'GET' },{ body: '{"error":["You must indicate a sql query"]}', - status: 500 + status: 400 }); }; @@ -71,7 +71,7 @@ tests['GET /api/v1/sql with SQL parameter on INSERT only. oAuth not used, so pub url: "/api/v1/sql?q=INSERT%20INTO%20untitle_table_4%20(id)%20VALUES%20(1)&database=cartodb_dev_user_1_db", method: 'GET' },{ - status: 500 + status: 400 }); }; @@ -80,7 +80,7 @@ tests['GET /api/v1/sql with SQL parameter on DROP DATABASE only. oAuth not used, url: "/api/v1/sql?q=DROP%20TABLE%20untitle_table_4&database=cartodb_dev_user_1_db", method: 'GET' },{ - status: 500 + status: 400 }); }; @@ -90,7 +90,7 @@ tests['GET /api/v1/sql with SQL parameter on INSERT only. header based db - shou headers: {host: 'vizzuality.cartodb.com'}, method: 'GET' },{ - status: 500 + status: 400 }); }; @@ -100,7 +100,7 @@ tests['GET /api/v1/sql with SQL parameter on DROP DATABASE only.header based db headers: {host: 'vizzuality.cartodb.com'}, method: 'GET' },{ - status: 500 + status: 400 }); };