Merge lp:~kyrofa/snappy-hub/snappy-debug_escape_regex_strings into lp:~snappy-dev/snappy-hub/snappy-debug

Proposed by Kyle Fazzari on 2015-11-13
Status: Merged
Merged at revision: 17
Proposed branch: lp:~kyrofa/snappy-hub/snappy-debug_escape_regex_strings
Merge into: lp:~snappy-dev/snappy-hub/snappy-debug
Diff against target: 74 lines (+12/-10)
1 file modified
bin/snappy-security-scanlog (+12/-10)
To merge this branch: bzr merge lp:~kyrofa/snappy-hub/snappy-debug_escape_regex_strings
Reviewer Review Type Date Requested Status
Jamie Strandboge 2015-11-13 Approve on 2015-11-13
Review via email: mp+277463@code.launchpad.net

Commit message

Escape externally-controlled strings in regex.

Description of the change

snappy-security-scanlog currently dies with a "sre_constants.error: multiple repeat" exception when an apparmor event contains things like "++" (e.g. libstdc++). This is because it builds regex based on externally-controlled strings (like event names) without escaping them.

This change simply escapes all externally-controlled strings provided to re.compile().

To post a comment you must log in.
Jamie Strandboge (jdstrand) wrote :

Erf, nice catch. Looks great, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/snappy-security-scanlog'
2--- bin/snappy-security-scanlog 2015-11-12 21:47:12 +0000
3+++ bin/snappy-security-scanlog 2015-11-13 16:31:39 +0000
4@@ -181,7 +181,7 @@
5 event.name is not None:
6 # capability rules
7 rule_re = re.compile(r'^\s*capability\s+%s\s*,\s*(#.*)?$' %
8- event.name)
9+ re.escape(event.name))
10
11 msg = "Capability: %s" % event.name
12 rec.append("adjust program to not require 'CAP_%s' (see 'man 7 "
13@@ -214,16 +214,16 @@
14 rec.append("adjust program to use relative paths if the snap "
15 "already ships '%s'" % bn)
16 exec_re = re.compile(r'^\s*%s\s+\w*x*\s*,\s*(#.*)?$' %
17- event.name)
18+ re.escape(event.name))
19 caps = self.apparmor_caps_allowing_rule(exec_re)
20 if caps is not None:
21 rec.append("add one of '%s' to 'caps'" % caps)
22 # try to match our alternation. This is where we need logprof
23- alt_exec_re = re.compile(r'^(/usr)?/s?bin/%s' % bn)
24+ alt_exec_re = re.compile(r'^(/usr)?/s?bin/%s' % re.escape(bn))
25 if alt_exec_re.search(event.name):
26 exec_re = re.compile(
27 r'^\s*/\{,usr/\}\{,s\}bin/%s\s+\w*x*\s*,\s*(#.*)?' %
28- bn)
29+ re.escape(bn))
30 caps = self.apparmor_caps_allowing_rule(exec_re)
31 if caps is not None:
32 rec.append("add one of '%s' to 'caps'" % caps)
33@@ -265,7 +265,8 @@
34 else:
35 for fn in [event.name, self._aa_file(event.name)]:
36 file_re = re.compile(r'^\s*%s\s+\w*%s\w*\s*,\s*(#.*)?$' %
37- (event.name, mask[0]))
38+ (re.escape(event.name),
39+ re.escape(mask[0])))
40 caps = self.apparmor_caps_allowing_rule(file_re)
41 if caps is not None:
42 rec.append("add one of '%s' to 'caps'" % caps)
43@@ -296,7 +297,8 @@
44 "'security-policy'" % (event.net_family, addr))
45 # FIXME: unix rules can span multiple lines
46 unix_re = re.compile(
47- r'^\s*unix\s+.*\s+addr=[\'"]?%s[\'"]?[\s,]?' % addr)
48+ r'^\s*unix\s+.*\s+addr=[\'"]?%s[\'"]?[\s,]?' %
49+ re.escape(addr))
50 caps = self.apparmor_caps_allowing_rule(unix_re)
51 if caps is not None:
52 rec.append("add one of '%s' to 'caps'" % caps)
53@@ -304,9 +306,9 @@
54 rec.append("add 'network %s %s,' to apparmor in "
55 "'security-policy'" % (event.net_family,
56 event.net_sock_type))
57- net_re = re.compile(
58- r'^\s*network\s+%s\s+%s\s*,\s*(# .*)?$' % (event.net_family,
59- event.net_sock_type))
60+ net_re = re.compile(r'^\s*network\s+%s\s+%s\s*,\s*(# .*)?$' %
61+ (re.escape(event.net_family),
62+ re.escape(event.net_sock_type)))
63 caps = self.apparmor_caps_allowing_rule(net_re)
64 if caps is not None:
65 rec.append("add one of '%s' to 'caps'" % caps)
66@@ -322,7 +324,7 @@
67 # TODO: Search releases other than 15.04, too
68 dirs = ["/usr/share/seccomp/policygroups/ubuntu-core/15.04",
69 "/var/lib/snappy/seccomp/policygroups"]
70- syscall_re = re.compile(r'^\s*%s\s*(#.*)?$' % syscall)
71+ syscall_re = re.compile(r'^\s*%s\s*(#.*)?$' % re.escape(syscall))
72 for cap_dir in dirs:
73 for cap in os.listdir(cap_dir):
74 if cap == "networking": # FIXME: not correct for personal

Subscribers

People subscribed via source and target branches