Eliminate SQL injection vulnerability in node_cache

In node_cache.find_node() we were constructing a raw SQL query using
unescaped data that came in on the wire. This presented an SQL injection
vulnerability. To avoid this, use the query builder from SQLAlchemy to
ensure that any input strings are correctly escaped.

Change-Id: I2b0ffa307ec1aa57538733f2e454d2d7e994d656
Story: #2005678
Task: 30992
This commit is contained in:
Zane Bitter 2019-05-14 15:39:19 -04:00 committed by Dmitry Tantsur
parent b8d1bda4c6
commit 9d107900b2
3 changed files with 18 additions and 9 deletions

View File

@ -18,6 +18,7 @@ import contextlib
import copy
import datetime
import json
import operator
from automaton import exceptions as automaton_errors
from ironicclient import exceptions
@ -30,7 +31,6 @@ from oslo_utils import timeutils
from oslo_utils import uuidutils
import six
from sqlalchemy.orm import exc as orm_errors
from sqlalchemy import text
from ironic_inspector.common.i18n import _
from ironic_inspector.common import ironic as ir_utils
@ -840,14 +840,11 @@ def find_node(**attributes):
LOG.debug('Trying to use %s of value %s for node look up',
name, value)
value_list = []
for v in value:
value_list.append("name='%s' AND value='%s'" % (name, v))
stmt = ('select distinct node_uuid from attributes where ' +
' OR '.join(value_list))
rows = (db.model_query(db.Attribute.node_uuid).from_statement(
text(stmt)).all())
found.update(row.node_uuid for row in rows)
query = db.model_query(db.Attribute.node_uuid)
pairs = [(db.Attribute.name == name) &
(db.Attribute.value == v) for v in value]
query = query.filter(six.moves.reduce(operator.or_, pairs))
found.update(row.node_uuid for row in query.distinct().all())
if not found:
raise utils.NotFoundInCacheError(_(

View File

@ -315,6 +315,11 @@ class TestNodeCacheFind(test_base.NodeTest):
self.assertRaises(utils.Error, node_cache.find_node,
bmc_address='1.2.3.4')
def test_input_filtering(self):
self.assertRaises(utils.NotFoundInCacheError,
node_cache.find_node,
bmc_address="' OR ''='")
class TestNodeCacheCleanUp(test_base.NodeTest):
def setUp(self):

View File

@ -0,0 +1,7 @@
---
security:
- |
Fixes insufficient input filtering when looking up a node by information
from the introspection data. It could potentially allow SQL injections
via the ``/v1/continue`` API endpoint. See `story 2005678
<https://storyboard.openstack.org/#!/story/2005678>`_ for details.