Incorrect regular expressions used for schema validation

Bug #1471158 reported by Florian Weimer on 2015-07-03
260
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Designate
Critical
Kiall Mac Innes
Juno
Critical
Kiall Mac Innes
Kilo
Critical
Kiall Mac Innes
OpenStack Security Advisory
Undecided
Unassigned

Bug Description

The regular expressions listed in designate/schema/format.py allow trailing "\n" characters because "$" matches "\n" at the end of the string.

Submitting a record creation request with "name" ending with "\n" currently results in an internal server, with the following traceback in the log file:

Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/oslo_messaging/rpc/dispatcher.py", line 142, in _dispatch_and_reply
    executor_callback))
  File "/usr/lib/python2.7/site-packages/designate/rpc.py", line 178, in _dispatch
    return super(RPCDispatcher, self)._dispatch(*args, **kwds)
  File "/usr/lib/python2.7/site-packages/oslo_messaging/rpc/dispatcher.py", line 186, in _dispatch
    executor_callback)
  File "/usr/lib/python2.7/site-packages/oslo_messaging/rpc/dispatcher.py", line 130, in _do_dispatch
    result = func(ctxt, **new_args)
  File "/usr/lib/python2.7/site-packages/designate/central/service.py", line 220, in wrapper
    result = f(self, *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/designate/central/service.py", line 194, in wrapper
    result = f(self, *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/designate/central/service.py", line 1119, in create_recordset
    context, domain, recordset, increment_serial=increment_serial)
  File "/usr/lib/python2.7/site-packages/designate/central/service.py", line 84, in wrapper
    **copy.deepcopy(kwargs))
  File "/usr/lib/python2.7/site-packages/designate/central/service.py", line 123, in wrapper
    self.storage.rollback()
  File "/usr/lib/python2.7/site-packages/oslo_utils/excutils.py", line 119, in __exit__
    six.reraise(self.type_, self.value, self.tb)
  File "/usr/lib/python2.7/site-packages/designate/central/service.py", line 118, in wrapper
    result = f(self, *args, **kwargs)
  File "/usr/lib/python2.7/site-packages/designate/central/service.py", line 1138, in _create_recordset_in_storage
    self._is_valid_recordset_name(context, domain, recordset.name)
  File "/usr/lib/python2.7/site-packages/designate/central/service.py", line 341, in _is_valid_recordset_name
    raise ValueError('Please supply a FQDN')
ValueError: Please supply a FQDN

If such additional checks are everywhere, the incorrect regular expressions should be harmless, and the security flag can be removed.

Downstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=1235655

Kiall Mac Innes (kiall) on 2015-07-03
Changed in designate:
assignee: nobody → Kiall Mac Innes (kiall)
Kiall Mac Innes (kiall) on 2015-07-03
Changed in designate:
milestone: none → liberty-2
importance: Undecided → Critical

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/198379

Changed in designate:
status: New → In Progress
Kiall Mac Innes (kiall) wrote :
information type: Private Security → Public Security
Florian Weimer (fweimer) wrote :

Kiall, you could use \Z instead of $, then you will not need a look-behind assertion.

Kiall Mac Innes (kiall) wrote :

Heh - Serves me right for using debuggex rather than a python shell to test the modification.. \Z exhibits *in debuggex* the same behavior as $ does[1], so I wrote it off and moved to a look ahead.

[1]: https://www.debuggex.com/r/qxcIX03btwTQGXJI

Changed in designate:
status: In Progress → Fix Committed

Reviewed: https://review.openstack.org/198379
Committed: https://git.openstack.org/cgit/openstack/designate/commit/?id=33510373af4f1351401187ab1a9f0fe90debda86
Submitter: Jenkins
Branch: stable/kilo

commit 33510373af4f1351401187ab1a9f0fe90debda86
Author: Kiall Mac Innes <email address hidden>
Date: Fri Jul 3 14:56:31 2015 +0100

    Ensure validations account for trailing newlines

    The regex's used for validation allowed "\n" terminated strings
    to pass, allowing invalid data through.

    Change-Id: Iae4c40ffab9951280bb6d67c2a7cb779dc7ddf0f
    Closes-Bug: 1471158

Reviewed: https://review.openstack.org/198380
Committed: https://git.openstack.org/cgit/openstack/designate/commit/?id=09d65fddf1abfe9d7a3f3e63b87a03bea06e61ce
Submitter: Jenkins
Branch: stable/juno

commit 09d65fddf1abfe9d7a3f3e63b87a03bea06e61ce
Author: Kiall Mac Innes <email address hidden>
Date: Fri Jul 3 14:56:31 2015 +0100

    Ensure validations account for trailing newlines

    The regex's used for validation allowed "\n" terminated strings
    to pass, allowing invalid data through.

    Change-Id: Iae4c40ffab9951280bb6d67c2a7cb779dc7ddf0f
    Closes-Bug: 1471158

Thierry Carrez (ttx) on 2015-07-29
Changed in designate:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2015-10-15
Changed in designate:
milestone: liberty-2 → 1.0.0
tags: added: security

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

Changed in ossa:
status: New → Incomplete

Oups, designate vulnerabilities are not managed by the vmt, thus I closed the OSSA task.

However, should this get a CVE afterall ?

Changed in ossa:
status: Incomplete → Won't Fix
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers