Heat denial of service through template-validate

Bug #1533729 reported by Sergey Kraynev
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mirantis OpenStack
Fix Released
High
Sergey Kraynev
5.0.x
Won't Fix
High
MOS Maintenance
5.1.x
Fix Committed
High
Sergii Rizvan
6.0.x
Fix Released
High
Sergii Rizvan
6.1.x
Fix Released
High
Sergii Rizvan
7.0.x
Fix Released
High
Sergii Rizvan
8.0.x
Fix Released
High
Sergey Kraynev

Bug Description

in service.py validate_template, we do an env.get_class bypassing
 the global_environment(), which ends up calling
 template_resource.generate_class, which wrongly defaults the get_template_file
 allowed schemas to "('file',)"

 https://github.com/openstack/heat/blob/master/heat/engine/service.py#L958
 https://github.com/openstack/heat/blob/master/heat/engine/resources/template_resource.py#L31

 The net result of this is that any call to template-validate which
 specifies type: foo.yaml will read that file from the filesystem of the
 heat service - this actually means template-validate calls which should
 fail work on typical devstack env's where the client and heat-engine are
 co-located (it took me a while to work out why!!)

 I've not figured out any way for this to be exploitable, but it definitely
 seems wrong that we allow user-provided paths to be read like this,
 and there could be some risk if folks could work out a way to make
 validation blow up with a stack-trace containing any file contents.

Link on original bug: https://bugs.launchpad.net/heat/+bug/1496277

CVE References

Changed in mos:
assignee: nobody → Oleksii Chuprykov (ochuprykov)
no longer affects: mos/9.0.x
information type: Private → Private Security
description: updated
description: updated
Revision history for this message
Sergey Kraynev (skraynev) wrote :

Patch for MOS 8.0 was merged https://review.fuel-infra.org/#/c/16092/

Revision history for this message
Sergii Rizvan (srizvan) wrote :
Revision history for this message
Sergii Rizvan (srizvan) wrote :
Revision history for this message
Sergii Rizvan (srizvan) wrote :
tags: added: on-verification
Revision history for this message
Sergii Rizvan (srizvan) wrote :

Verified on MOS 6.0
Packages:
heat-engine
Version:
2014.2-fuel6.0~mira13

Revision history for this message
Alexander Nagovitsyn (gluk12189) wrote :

as I can see on 8.0 this error fixed

For the future - need more detailed information/description for reproducing this bug

information type: Private Security → Public Security
Dmitry (dtsapikov)
tags: removed: on-verification
tags: added: on-verification
Revision history for this message
Fuel Devops McRobotson (fuel-devops-robot) wrote : Fix merged to openstack/heat (openstack-ci/fuel-7.0/2015.1.0)

Reviewed: https://review.fuel-infra.org/16490
Submitter: Vitaly Sedelnik <email address hidden>
Branch: openstack-ci/fuel-7.0/2015.1.0

Commit: 63e14f24287f26ed6d5a84eb296a04685730ff59
Author: Zane Bitter <email address hidden>
Date: Wed Jan 27 13:48:51 2016

Load template files only from their known source

Modify get_class to ensure that user-defined resources cannot result in
reads from the local filesystem. Only resources defined by the operator
in the global environment should read local files.

To make this work, this patch also adds a separate
get_class_to_instantiate() method to the Environment.

We were previously using get_class for two different purposes - to get a
resource plugin on which we could perform introspection to obtain the
properties and attributes schema, and to get a resource plugin we could
instantiate to create a Resource object. These are both the same except in
the case of a TemplateResource, where having two different use cases for
the same piece of code was adding considerable extra complexity. Combining
the use cases in this way also made the error handling confusing (leading
to bug 1518458).

This change separates out the two cases.

Change-Id: I845e7d23c73242a4a4c9c40599690ab705c75caa
Closes-Bug: #1533729

Revision history for this message
Dmitry (dtsapikov) wrote :

Verified on 6.1+mu5

tags: removed: on-verification
Revision history for this message
Alexey Stupnikov (astupnikov) wrote :

Set this bug's status to Won't fix (wontfix-munotapplic) for 5.0 branch since we don't provide MU for 5.0 branch

tags: added: wontfix-munotapplic
Dmitry (dtsapikov)
tags: added: on-verification
Revision history for this message
Dmitry (dtsapikov) wrote :

Verified on 7.0+mu3

tags: removed: on-verification
Revision history for this message
Fuel Devops McRobotson (fuel-devops-robot) wrote : Fix merged to openstack/heat (openstack-ci/fuel-5.1.1-updates/2014.1.1)

Reviewed: https://review.fuel-infra.org/16792
Submitter: Vitaly Sedelnik <email address hidden>
Branch: openstack-ci/fuel-5.1.1-updates/2014.1.1

Commit: 9e9eead7afecbde724b69ae4049ac5dc61b98582
Author: Zane Bitter <email address hidden>
Date: Tue Feb 9 14:04:46 2016

Load template files only from their known source

Add a separate get_class_to_instantiate() method to Environment to
ensure that user-defined resources cannot result in reads from the local
filesystem. Only resources defined by the operator in the global
environment should read local files.

We were previously using get_class for two different purposes - to get a
resource plugin on which we could perform introspection to obtain the
properties and attributes schema, and to get a resource plugin we could
instantiate to create a Resource object. These are both the same except in
the case of a TemplateResource, where having two different use cases for
the same piece of code was adding considerable extra complexity. Combining
the use cases in this way also made the error handling confusing (leading
to bug 1518458).

This change separates out the two cases.

Change-Id: I845e7d23c73242a4a4c9c40599690ab705c75caa
Closes-Bug: #1533729

Jeremy Stanley (fungi)
description: updated
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.