Skip to content

Commit

Permalink
Add query var scrubbing lib
Browse files Browse the repository at this point in the history
Add reg_scrub.py, to cleanse user input prior to use.  Mostly checking
for invalid characters in the values, and malformed variable names right
now.

Rearrange a touch in preparation for next usage check (del being the
only action if requested).
  • Loading branch information
briangerard committed Aug 5, 2016
1 parent 6e30fe2 commit c0a79de
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 7 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
EXES = register process_registrations
LIBS = reg_form.py reg_request_db.py member.py process_form.py
LIBS = reg_form.py reg_request_db.py reg_scrub.py member.py process_form.py
SRCS = $(EXES) $(LIBS)

DSTDIR = /usr/lib/cgi-bin/
Expand Down
17 changes: 11 additions & 6 deletions process_registrations
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

import cgi
import cgitb
import html
import re
import sqlite3

import member
import process_form
import reg_scrub
from reg_request_db import RegDb

debugdir = '/tmp'
Expand Down Expand Up @@ -54,7 +54,13 @@ else:
print(process_form.footer)

else:
deletions = (var for var in the_form if 'del_' in var)
cleaned_form = reg_scrub.sanitize(the_form)
#print("<strong>{}</strong></br>".format(cleaned_form))

deletions = list(var for var in cleaned_form if 'del_' in var)
modifications = list(var for var in cleaned_form if 'mod_' in var)
registrations = list(var for var in cleaned_form if 'reg_' in var)

canceled = {}
for deletion in deletions:
reqid = (deletion.split('_'))[-1]
Expand All @@ -66,21 +72,20 @@ else:
else:
print("<strong>Request id {} is gawn.</strong></br>".format(reqid))

modifications = (var for var in the_form if 'mod_' in var)
for mod in modifications:
reqid = (mod.split('_'))[-1]
if not re.search(r'\D', reqid) and reqid not in canceled:
fields = member.ordered_field_names()
for field in fields:
val = the_form[field+'_'+reqid].value
val = cleaned_form[field+'_'+reqid]
try:
reg_db.modify(reqid, field, val)
except RuntimeError as rte:
print("<strong>No can modify: {} (id: {})</strong></br>".format(rte, reqid))
print("<strong>Unable to modify: {} (id: {})</strong></br>".format(rte, reqid))
else:
print("<strong>Modified id {}, field {}. Now: {}</strong></br>".format(
reqid, field, val))
else:
print("<strong>Cannot modify request id {}: malformed or deleted.</strong></br>".format(reqid))
print("<strong>Unable to modify request id {}: malformed or deleted.</strong></br>".format(reqid))

# registrations = ...
41 changes: 41 additions & 0 deletions reg_scrub.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import html
import member
import re

_allowed_fields = member.ordered_field_names()
_allowed_fields.extend(('del', 'mod', 'reg'))

def sanitize(web_form):
'''Take a cgi.FieldStorage object and scrub it down to sane values.
returns a dict with the cgi vars as keys and their html.escape()ed
values as the values.'''

# TODO: collect all vars for a given reqid - if the full set's vars
# don't pass muster, drop that reqid entirely.
clean_vars = {}
for var in web_form:
if name_ok(var):
if val_ok(web_form[var].value):
clean_vars[var] = html.escape(web_form[var].value)
return clean_vars

def name_ok(name):
'''Check that this is a legal name for a variable in the form.
The name part must be one of the fields from above, and the reqid
must consist of only digits.'''
varparts = name.split('_')
if len(varparts) == 2:
if varparts[0] in _allowed_fields:
if not re.search(r'\D', varparts[1]):
return True
return False

def val_ok(string):
'''Check that this is a valid value from the form.'''

# If any character besides those listed in the character class below
# are in the string, then we call shenanigans. Otherwise, we'll say
# it's legal.
if not re.search(r'[^-a-zA-Z0-9_.@# \']', string):

This comment has been minimized.

Copy link
@aschrab

aschrab Aug 12, 2016

Member

I'm pretty sure Cristóbal (and likely others) would have an issue with this.

This comment has been minimized.

Copy link
@briangerard

briangerard Aug 14, 2016

Author Contributor

Yeah, likely so. This was definitely just the first cut at it, to give us some basic validation to get up and running by the meeting. :-) I just filed issue #7 to look at replacing val_ok with something a bit more full-featured. Feel free to add comments and suggestions there!

return True
return False

0 comments on commit c0a79de

Please sign in to comment.