stack_create 500 Error with non-string stack_name

[Ben Ramsey]

When sending a stack create request using the REST API through curl with the stack_name parameter set to either a list, or a dictionary a Server Error occurs.

The major problem with this is that when this occurs the response of the request will show what database backend the cloud is utilising whether it be MySQL, or PostgreSQL. This information could be used by a malicious user to exploit the databases. Additionally to the database engine being known, the query itself is also printed in the response.

This occurs regardless of the level of logging being set for the heat service.

This has been seen in the master branch as of 12/01/2016.

I have attached the response of a request with a Traceback.

The data being sent is the following:
{
    "files": {},
    "disable_rollback": true,
    "parameters": {
        "flavor": "m1.tiny"
    },
    "stack_name": {"test": "sample"},
    "template": {
        "heat_template_version": "2013-05-23",
        "description": "Simple template to test heat commands",
        "parameters": {
            "flavor": {
                "default": "m1.tiny",
                "type": "string"
            }
        },
        "resources": {
            "hello_world": {
                "type": "OS::Nova::Server",
                "properties": {
                    "key_name": "heat_key",
                    "flavor": {
                        "get_param": "flavor"
                    },
                    "image": "40be8d1a-3eb9-40de-8abd-43237517384f",
                    "user_data": "#!/bin/bash -xv\necho \"hello world\" > /root/hello-world.txt\n"
                }
            }
        }
    },
    "timeout_mins": 60
}

[Tristan Cacqueray (tristan-cacqueray)]

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.

The database schema is already known, I'm not sure leaking the database backend type is really an issue.

[Ben Ramsey (benjamin-james-ramsey)]

In terms of leaking the database backend type, this by itself might not be an issue but it could lead to giving an attacker knowledge that they should not have. This could lead to a more focused attack by them utilising exploits in the particular engine.

Aside from this, the bug highlights that there is data being sent to the database before it has been sanitised, or validated which could be an issue.

[Steven Hardy (shardy)]

Hi thanks for the report - it certainly looks like a bug (working to reproduce locally).

Regarding the security risk, can you clarify a couple of things please:

1. What in the (bad) response data identifies the DB backend, vs just that we're using sqlalchemy?
2. Is there anything in the response which couldn't also be derived by looking at the code and/or testing on another openstack environment e.g devstack?
3. Does the error response ever contain any connection details or credentials which would enable malicious access to the DB? (I can't see any atm?)
4. Can you outline any specific exploit related to this information exposure, vs just a general risk that it provides more information about the environment to a hostile user?

Just trying to clarify the level and nature of the security risk, thanks!

[Steven Hardy (shardy)]

Initial patch to master, needs tests

[Steven Hardy (shardy)]

Second draft of the patch, now includes tests.

[Zane Bitter (zaneb)]

+2 for the patch

For the record, I don't accept at all that this should be a private security issue. The exception is from SQLAlchemy, and the only information it leaks that isn't available from the public source code is whether the DB is MySQL or Postgresql - something that the user could guess with 95% accuracy in one attempt, or 100% accuracy in 2 attempts.

[Ben Ramsey (benjamin-james-ramsey)]

@shardy - the patch seems to work well, here are my answers to those questions if still required.

1. In the error message SQLAlchemy shows that the DBError is of a type (<library>.ProgrammingError) the library being either one associated with Postgresql, or MySQL.

2. While the default is MySQL, it is possible to also deploy with Postgresql, so this is deployment specific and thus not visible from source code

3. This did not occur in any of my testing

4. I am unable to come up with a specific exploit at this point in time for this issue.

@zaneb - While I agree that the information can be guessed by the user fairly readily, the fact that data was being sent to the database without checking did warrant further investigation for security reasons.

[Steven Hardy (shardy)]

Ok, let's await feedback from the security response team, but IMHO given the feedback in #8, I think this can probably be handled as a public bug (probably public security)

While it's true the current code does pass the information into the DB API without checking, all it does is use it for filter_by:

https://github.com/openstack/heat/blob/master/heat/db/sqlalchemy/api.py#L340

So, I think, there's no way for this to be used to manipulate the actual query, you'll either blow up the query due to invalid type (as in this report), or return nothing (with my fix applied, because we force everything to a string).

[Jeremy Stanley (fungi)]

Based on the discussion above, this sounds like class C1 (not considered a practical vulnerability https://security.openstack.org/vmt-process.html#incident-report-taxonomy ) so I'm switching the security advisory task to Won't Fix and triaging this in public as a hardening opportunity. If new details emerge, this decision can be revisited.

[Steven Hardy (shardy)]

Per comment #10, I'll post the proposed fix to gerrit and we can use the normal review process to land it.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/267631

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/267631
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=1636b3b0530f2e8422997fff810188e4676de511
Submitter: Jenkins
Branch: master

commit 1636b3b0530f2e8422997fff810188e4676de511
Author: Steven Hardy <email address hidden>
Date: Wed Jan 13 10:34:18 2016 +0000

    Handle invalid stack names which are non-string

    If we get passed a non-string stack name, e.g a map or list, we
    fail with a DB error associated with looking up the existing stack.

    So instead force all stack lookups to use string identifiers, and
    make the name validation for new stacks robust to fail gracefully
    when there is an invalid (non string) argument passed.

    Change-Id: I052dc4a715773895d070e1e9f26183c6a1cf3d7f
    Closes-Bug: #1533065

[Thierry Carrez (ttx)]

This issue was fixed in the openstack/heat 6.0.0.0b2 development milestone.

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/heat 6.0.0.0b2 development milestone.