redos of nfs_loc_pattern

Bug #2047686 reported by lujiefsi
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
New
Undecided
Unassigned
OpenStack Security Advisory
Won't Fix
Undecided
Unassigned

Bug Description

What is redos : https://codeql.github.com/codeql-query-help/python/py-redos/

a redos at https://github.com/openstack/cinder/blob/71c021c501489840f92e60ba6bcce5532b451dc8/cinder/volume/drivers/netapp/dataontap/nfs_base.py#L855C13-L855C29

poc
```
import re
import datetime

nfs_loc_pattern = \
                (r'^nfs://(([\w\-\.]+:{1}[\d]+|[\w\-\.]+)(/[^\/].*)'
                 r'*(/[^\/\\\\]+)$)')

def split(x):
  string="nfs://-/." + "/."*x +"/"
  starttime = datetime.datetime.now()
  matched = re.match(nfs_loc_pattern, string, flags=0)
  endtime = datetime.datetime.now()
  print ("string length = " + str(x) + " time cost=" + str((endtime - starttime).seconds) + " seconds")

split(3)
split(30)
split(300)
split(3000)
```

Tags: security
lujiefsi (lujiefsi)
description: updated
Revision history for this message
Jeremy Stanley (fungi) wrote :

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.

It's not clear to me that an attacker would have the ability to pass a nefarious NFS path to the Cinder NetApp driver, but hopefully the Cinder security reviewers can confirm whether this is possible for anyone other than service administrators to exploit.

description: updated
Changed in ossa:
status: New → Incomplete
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

iiuc, the vulnerability is triggered if an nfs:// URL that exploits the regex inefficiency is an image location in glance

I think this can be worked in public as a hardening opportunity for the following reasons (numbered for easy reference by people checking my reasoning here):

1. this can only occur under a specific configuration of cinder (and I think, glance; both must be using an nfs backend, and cinder must be using a netapp nfs backend)

2. if such nfs:// URLs were used in normal operation, this inefficiency would have been discovered already because of undesirable pauses during volume creation

3. thus, an attacker would have to get the code in a position where it processes a "bad" URL

4. the code path occurs in cloning a volume from an image, when the image location is parsed to see if it is usable by the backend

5. OSSN-0090 recommends that end users not be allowed to directly set image locations

6. (further, even if OSSN-0090 is ignored, if the nfs backend isn't enabled in glance, I'm pretty sure the "nfs://" location scheme is rejected and can't be set as an image location)

So, I don't think this exploit should occur "in the wild". That said, I have no objection against hardening the netapp driver; I just think it can be done in public in the normal way.

Revision history for this message
Jeremy Stanley (fungi) wrote :

Thanks for looking into this, Brian. I've switched the report to public now and tagged it as a potential hardening opportunity, class D (or possibly C1) in our taxonomy: https://security.openstack.org/vmt-process.html#report-taxonomy

description: updated
information type: Private Security → Public
tags: added: security
Changed in ossa:
status: Incomplete → Won't Fix
Revision history for this message
Rajat Dhasmana (whoami-rajat) wrote :

Looking at the whole code around this in the netapp driver, this looks like legacy support for old netapp arrays but the netapp team can correct me.

The "creating a bootable volume from image operation" workflow starts with cinder calling clone_image[1] method of netapp nfs driver for efficient cloning.

Netapp driver tries to
1. finds the image in the cache (not image volume cache)[2]
2. directly clone it from mountpoint[1]

1. the cache is a file named img-cache-<image-id> in one of the nfs mounted shares[3]

Concern: this img-cache-<image-id> file is written when creating a bootable volume from image and is only possible with "earliest versions of FlexGroup[4]" as stated in the comment, which makes me feel this code is kept for backward compatibility.

2. checks if the image metadata has (type=nfs, share_location and mountpoint). If all 3 exist then continues with cloning[5]

Concern: not really sure who sets these properties in the image

And on top of all this, I'm not even sure if nfs:// prefix is used by any glance image. I'm aware that filesystem backend of glance store is used to emulate nfs behavior but that uses "file://" as prefix[6].

I agree with Brian's analysis that this can be treated as hardening opportunity but my concern is if the code in question is even used at this point and maybe a target for potential cleanup?

[1] https://github.com/openstack/cinder/blob/71c021c501489840f92e60ba6bcce5532b451dc8/cinder/volume/drivers/netapp/dataontap/nfs_base.py#L644
[2] https://github.com/openstack/cinder/blob/71c021c501489840f92e60ba6bcce5532b451dc8/cinder/volume/drivers/netapp/dataontap/nfs_base.py#L674-L679
[3] https://github.com/openstack/cinder/blob/71c021c501489840f92e60ba6bcce5532b451dc8/cinder/volume/drivers/netapp/dataontap/nfs_base.py#L533
[4] https://github.com/openstack/cinder/blob/71c021c501489840f92e60ba6bcce5532b451dc8/cinder/volume/drivers/netapp/dataontap/nfs_base.py#L509-L511
[5] https://github.com/openstack/cinder/blob/71c021c501489840f92e60ba6bcce5532b451dc8/cinder/volume/drivers/netapp/dataontap/nfs_base.py#L917-L923
[6] https://github.com/openstack/glance_store/blob/6f5011d1f05c99894fb8b909d33ad23a20bf83a9/glance_store/_drivers/filesystem.py#L214

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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