murano team mailing list archive

[Jeremy Stanley <fungi@xxxxxxxxxxx>]

Note that this still leaves a bit of a landmine in the ReddwarfException
subclass, since it passes the first argument through into
OpenstackException's message string. Compare with NovaException and
CinderException which skip string formatting if you try to do that.

The current convention for official OpenStack projects moving to
OpenstackException is to disallow arbitrary exception strings passed in
raise statements and always override the message format string with
another string constant specific to the subclass, which is the safest
way to go about avoiding such risks in the future (for an example have a
look at the way QuantumException is implemented).

I'll open a separate Reddwarf bug with this hardening recommendation,
and one for Glance as well (GlanceException does something similarly
risky, formatting the message even if it came in from a raise).  I'll
also open a bug on Oslo to get some additional cautionary wording in
place in the docstring for OpenstackException highlighting this.
Further, I'll start a thread on the openstack-security list to see if
the OSSG has any interest in organizing a format string vulnerability
audit, checking official projects for any similarly risky string
formatting issues.

-- 
You received this bug notification because you are a member of Murano
Core, which is subscribed to murano.
Matching subscriptions: team
https://bugs.launchpad.net/bugs/1177936

Title:
  Format string vulnerability in base OpenstackException handler

Status in Cinder:
  Invalid
Status in OpenStack Image Registry and Delivery Service (Glance):
  Invalid
Status in Orchestration API (Heat):
  Invalid
Status in Moniker:
  Invalid
Status in Murano Project:
  Invalid
Status in OpenStack Compute (Nova):
  Invalid
Status in Oslo - a Library of Common OpenStack Code:
  Invalid
Status in OpenStack Quantum (virtual network service):
  Invalid
Status in RedDwarf - Database as a Service:
  Fix Committed
Status in Savanna project:
  Invalid

Bug description:
  Description

  A Reddwarf string format vulnerability exists in the class
  OpenstackException, the base exception class. To implement the class,
  as the comments in the class note, "inherit from it and define a
  'message' property. That message will get printf'd with the keyword
  arguments provided to the constructor". However, in the event that the
  message property contains user-supplied data, a user can inject %
  formatting string options to be printf'd.

  Affected Services

  Other projects in or related to OpenStack were found using either the
  same base exception handler, or one very similar.

  Nova – Class NovaException (optionally formats a string)
  Quantum – Classes QuantumException & OpenstackException
  Reddwarf – Class OpenstackException
  Moniker – Class OpenstackException
  Savanna – Class OpenstackException
  Murano-Conductor - Class OpenstackException

  Proof of Concept

  The following will demonstrate the issue by using the RedDwarf web
  service API to create a database with an invalid database name. This
  causes a ValueError to occur which uses the database name as part of
  the error message. When this is caught a BadRequest exception is
  generated which is then and handled by a ReddwarfError in the web
  service. Since ReddwarfError is a subclass of OpenstackException it
  sets the global message variable that is then used to create the
  format string.

  {
      "databases": [
          {
              "character_set": "utf8",
              "collate": "utf8_general_ci",
              "name": "%-100sa"
          }
      ]
  }

  In this case the API will send back to the client a HTTP 400 bad
  request with the following response body:

  {"badRequest": {"message": "'{}
  a' is not a valid database name", "code": 400}}

  The message contains a "{}", 100 space characters and the character
  'a'. The "{}" is due to kwargs being empty, -100 left-justifies the
  string by padding 100 space characters followed by the 'a' character.
  Since the padding takes an int value, a malicious user could include a
  very large int causing a denial of service. While running the unit
  tests for Reddwarf it was found that upon trying to create a database
  with the name "test%999999999sdb", python allocated more than 2000
  megabytes to handle a single request.

  Recommendation

  A class should not blindly format a string using a formatting variable
  without being able to validate the input. Instead of formatting the
  string, the exception base handler should take a string message
  argument while subclasses take care of formatting any needed string to
  pass into the super class.

To manage notifications about this bug go to:
https://bugs.launchpad.net/cinder/+bug/1177936/+subscriptions