[Mpuls-commits] r4914 - in base/trunk: . mpulsweb/controllers mpulsweb/lib mpulsweb/templates/evaluation
scm-commit@wald.intevation.org
scm-commit at wald.intevation.org
Fri Apr 15 16:42:50 CEST 2011
Author: bh
Date: 2011-04-15 16:42:47 +0200 (Fri, 15 Apr 2011)
New Revision: 4914
Modified:
base/trunk/ChangeLog
base/trunk/mpulsweb/controllers/case_bundle.py
base/trunk/mpulsweb/controllers/evaluate.py
base/trunk/mpulsweb/controllers/evaluation_overview.py
base/trunk/mpulsweb/lib/validators.py
base/trunk/mpulsweb/templates/evaluation/evaluate.mako
base/trunk/mpulsweb/templates/evaluation/overview.mako
Log:
Remove the sql_where form parameter to avoid SQL injections.
Fixes part of mpuls/issue1961
The solution is to store the sql_where text in the session. This
requires two keys in the session, one to remember the where clause
from the last search (key 'sql_where_candidate') and another to
store that candidate value when the user selects the 'Evaluate'
action on the overview page (key 'sql_where'). This is needed to
make sure that the value is kept even if the user performs another
search between the 'Evaluate' action and starting an evaluation.
If the user performs two searches in parallel, e.g. on two
different tabs in the browser, the evalaute Action may end up
storing the wrong sql_where value, but we can have to live with
that as it's not a recommended way to work with mpuls anyway.
* mpulsweb/controllers/case_bundle.py
(CaseBundleController.bundleAction): Copy sql_where_candidate to
sql_where in the session data.
* mpulsweb/controllers/evaluate.py (get_search_options): Remove
parameter sql_where. That parameter is not a form parameter
anymore.
(EvaluateController.evaluate): Adapt to get_search_options change.
(EvaluateController._get_evalparams): Add parameter
sql_where. That parameter is not in the form_result, but is part
of the parameters that will be passed to libmpuls, so it needs to
be passed separately.
(EvaluateController.evaluateAction): Pass sql_where taken from the
session data. Also, revert the temporary solution from r4814 for
the SQL-injection problem as it's not necessary anymore, and was
far too strict anyway.
* mpulsweb/controllers/evaluation_overview.py
(EvaluationOverviewController.overview): Save the SQL where
expression used in the session under the key
'sql_where_candidate'.
(EvaluationOverviewController.bundleAction): Copy
sql_where_candidate to sql_where in the session data.
* mpulsweb/lib/validators.py (EvaluationFormValidator.sql_where)
(BundleActionForm.sql_where): Removed. Not used anymore.
* mpulsweb/templates/evaluation/overview.mako,
mpulsweb/templates/evaluation/evaluate.mako: Remove hidden input
field sql_where. It's not used anymore.
Modified: base/trunk/ChangeLog
===================================================================
--- base/trunk/ChangeLog 2011-04-15 11:10:32 UTC (rev 4913)
+++ base/trunk/ChangeLog 2011-04-15 14:42:47 UTC (rev 4914)
@@ -1,5 +1,54 @@
2011-04-15 Bernhard Herzog <bh at intevation.de>
+ Remove the sql_where form parameter to avoid SQL injections.
+ Fixes part of mpuls/issue1961
+
+ The solution is to store the sql_where text in the session. This
+ requires two keys in the session, one to remember the where clause
+ from the last search (key 'sql_where_candidate') and another to
+ store that candidate value when the user selects the 'Evaluate'
+ action on the overview page (key 'sql_where'). This is needed to
+ make sure that the value is kept even if the user performs another
+ search between the 'Evaluate' action and starting an evaluation.
+
+ If the user performs two searches in parallel, e.g. on two
+ different tabs in the browser, the evalaute Action may end up
+ storing the wrong sql_where value, but we can have to live with
+ that as it's not a recommended way to work with mpuls anyway.
+
+ * mpulsweb/controllers/evaluation_overview.py
+ (EvaluationOverviewController.overview): Save the SQL where
+ expression used in the session under the key
+ 'sql_where_candidate'.
+ (EvaluationOverviewController.bundleAction): Copy
+ sql_where_candidate to sql_where in the session data.
+
+ * mpulsweb/controllers/evaluate.py (get_search_options): Remove
+ parameter sql_where. That parameter is not a form parameter
+ anymore.
+ (EvaluateController.evaluate): Adapt to get_search_options change.
+ (EvaluateController._get_evalparams): Add parameter
+ sql_where. That parameter is not in the form_result, but is part
+ of the parameters that will be passed to libmpuls, so it needs to
+ be passed separately.
+ (EvaluateController.evaluateAction): Pass sql_where taken from the
+ session data. Also, revert the temporary solution from r4814 for
+ the SQL-injection problem as it's not necessary anymore, and was
+ far too strict anyway.
+
+ * mpulsweb/controllers/case_bundle.py
+ (CaseBundleController.bundleAction): Copy sql_where_candidate to
+ sql_where in the session data.
+
+ * mpulsweb/lib/validators.py (EvaluationFormValidator.sql_where)
+ (BundleActionForm.sql_where): Removed. Not used anymore.
+
+ * mpulsweb/templates/evaluation/overview.mako,
+ mpulsweb/templates/evaluation/evaluate.mako: Remove hidden input
+ field sql_where. It's not used anymore.
+
+2011-04-15 Bernhard Herzog <bh at intevation.de>
+
* mpulsweb/controllers/evaluate.py: Fix formatting.
2011-04-15 Bernhard Herzog <bh at intevation.de>
Modified: base/trunk/mpulsweb/controllers/case_bundle.py
===================================================================
--- base/trunk/mpulsweb/controllers/case_bundle.py 2011-04-15 11:10:32 UTC (rev 4913)
+++ base/trunk/mpulsweb/controllers/case_bundle.py 2011-04-15 14:42:47 UTC (rev 4914)
@@ -193,6 +193,7 @@
session['casebundle'] = case_bundle
session["evaluation_ids"] = case_bundle.listDatasetIds()
session["id_field"] = form_result.get('id_field')
+ session["sql_where"] = session.get('sql_where_candidate')
session.save()
c.url_ok = url_for(controller='/case_overview')
num = len(case_bundle)
Modified: base/trunk/mpulsweb/controllers/evaluate.py
===================================================================
--- base/trunk/mpulsweb/controllers/evaluate.py 2011-04-15 11:10:32 UTC (rev 4913)
+++ base/trunk/mpulsweb/controllers/evaluate.py 2011-04-15 14:42:47 UTC (rev 4914)
@@ -48,12 +48,11 @@
return sdate, edate
-def get_search_options(soptions=None, id=None, selected_ids=(), id_field=None, sql_where=None):
+def get_search_options(soptions=None, id=None, selected_ids=(), id_field=None):
options = {}
options['id'] = id
options["selected_ids"] = selected_ids
options["id_field"] = id_field
- options["sql_where"] = sql_where
options['typelist'] = c.evalconfig.get_evaluations()
# set default evaluation options.
@@ -92,8 +91,7 @@
None, None, None, None, None)
evaloptions = get_search_options(session.get('evaluation.options'), id,
session.get("evaluation_ids", ()),
- session.get("id_field"),
- session.get("sql_where"))
+ session.get("id_field"))
# If user selects adele-evaluation render page with disabled
# configuration elements. Change default params
@@ -110,7 +108,7 @@
return formencode.htmlfill.render(form, defaults=defaults,
errors={}, auto_insert_errors=False)
- def _get_evalparams(self, form_result):
+ def _get_evalparams(self, form_result, sql_where):
params = {}
params['id'] = form_result['id']
params['start_date'] = str(form_result['start_date'])
@@ -160,11 +158,11 @@
% (id_field,
",".join(map(str, form_result["selected_ids"]))))
- if form_result.get("sql_where"):
- where_clauses.append(form_result.get("sql_where"))
-
where_clauses.append(selected_ids)
+ if sql_where:
+ where_clauses.append(sql_where)
+
where_clauses.append(self.type_ending_clause(params))
params['sql'] = ("SELECT %%(fields)s from master_tbl_eval_total_view m"
@@ -213,7 +211,8 @@
auto_insert_errors=False)
# Build evaluation
- eval_params = self._get_evalparams(form_result)
+ eval_params = self._get_evalparams(form_result,
+ session.get("sql_where"))
try:
try:
conn = db.getConnection()
@@ -221,13 +220,7 @@
eval_params['start_date'],
eval_params['end_date'],
None, None, None,
-
- # issue1961: SQL-injection possible
- # through parameter, so set to None
- # for the moment (application server)
- #eval_params['sql'],
- None,
-
+ eval_params['sql'],
eval_params['typelist'])
evalset = EvaluationSet(evalconfig)
evalset.evaluate()
Modified: base/trunk/mpulsweb/controllers/evaluation_overview.py
===================================================================
--- base/trunk/mpulsweb/controllers/evaluation_overview.py 2011-04-15 11:10:32 UTC (rev 4913)
+++ base/trunk/mpulsweb/controllers/evaluation_overview.py 2011-04-15 14:42:47 UTC (rev 4914)
@@ -143,7 +143,8 @@
form_defaults["id_field"] = "m.fkz::integer"
form_defaults["all_ids"] = " ".join(unicode(case.id)
for case in c.cases.getDatasets() if case.id)
- form_defaults["sql_where"] = c.cases.getWhereSQL()
+ session["sql_where_candidate"] = c.cases.getWhereSQL()
+ session.save()
return formencode.htmlfill.render(overview, defaults=form_defaults,
errors=c.form_errors)
@@ -209,7 +210,7 @@
if action == 'evaluate':
session["evaluation_ids"] = form_result.get('case_id')
session["id_field"] = form_result.get('id_field')
- session["sql_where"] = form_result.get('sql_where')
+ session["sql_where"] = session.get("sql_where_candidate")
if form_result.get('all_cases') > 0:
session["evaluation_ids"] = form_result.get('all_ids')
session.save()
Modified: base/trunk/mpulsweb/lib/validators.py
===================================================================
--- base/trunk/mpulsweb/lib/validators.py 2011-04-15 11:10:32 UTC (rev 4913)
+++ base/trunk/mpulsweb/lib/validators.py 2011-04-15 14:42:47 UTC (rev 4914)
@@ -672,7 +672,6 @@
type_ending = ForEach(String(), convert_to_list=True)
phase = ForEach(String(), convert_to_list=True)
show_percent = Bool(if_missing=False)
- sql_where = String(if_missing=None)
id_field = String(if_missing=None)
selected_ids = Wrapper(to_python=lambda s: [int(item)
for item in s.split()],
@@ -708,7 +707,6 @@
'evaluate']))
case_id = ForEach(Int(), convert_to_list=True)
id_field = String(if_missing=None)
- sql_where = String(if_missing=None)
all_ids = Wrapper(to_python=lambda s: [int(item) for item in s.split()])
Modified: base/trunk/mpulsweb/templates/evaluation/evaluate.mako
===================================================================
--- base/trunk/mpulsweb/templates/evaluation/evaluate.mako 2011-04-15 11:10:32 UTC (rev 4913)
+++ base/trunk/mpulsweb/templates/evaluation/evaluate.mako 2011-04-15 14:42:47 UTC (rev 4914)
@@ -71,7 +71,6 @@
len(evaluation_ids)) % len(evaluation_ids))}
<input type="hidden" name="selected_ids">
<input type="hidden" name="id_field">
- <input type="hidden" name="sql_where">
</td>
</tr>
% endif
Modified: base/trunk/mpulsweb/templates/evaluation/overview.mako
===================================================================
--- base/trunk/mpulsweb/templates/evaluation/overview.mako 2011-04-15 11:10:32 UTC (rev 4913)
+++ base/trunk/mpulsweb/templates/evaluation/overview.mako 2011-04-15 14:42:47 UTC (rev 4914)
@@ -24,7 +24,6 @@
${ungettext('Select the %s found agency.', 'Select all %s found agencys', c.count_all) % c.count_agency}</label>
<input type="hidden" name="all_ids" value="">
<input type="hidden" name="id_field" value="">
- <input type="hidden" name="sql_where" value="">
</div>
<div class="waska_form_element w30">
<select name="action">
More information about the Mpuls-commits
mailing list