Bug #14635

CVE-2016-3693 - `inspect` in a provisioning template exposes sensitive controller information

Added by Dominic Cleal over 1 year ago. Updated over 1 year ago.

Status:Closed
Priority:High
Assigned To:Ivan Necas
Category:Security
Target version:-
Difficulty: Bugzilla link:
Found in release: Pull request:https://github.com/theforeman/foreman/pull/3430
Story points-
Velocity based estimate-
Release1.11.1Release relationshipAuto

Description

A provisioning template containing <%= inspect %> will expose sensitive information about the Rails controller and application when rendered when using Safemode rendering (default).

Safemode is initialised with a "delegate" object that is typically the Rails controller. When inspect is called on it, all information about the Rails app is exposed, including routes, secret tokens, caches and so on.

Thanks to Ivan Necas for reporting the security issue to .

All versions of Foreman are vulnerable, CVE identifier will be requested.

Associated revisions

Revision 82f9b93c
Added by Ivan Necas over 1 year ago

Fixes #14635 - bump safemode version to fix the unwanted inspect issue

Revision c4e5d9a2
Added by Dominic Cleal over 1 year ago

refs #14635 - require safemode 1.2.4

History

#1 Updated by Dominic Cleal over 1 year ago

I'd suggest the rendering methods shouldn't be mixed directly into controllers and should instead be in a more isolated object, which would limit the amount of data being exposed.

It may be worth trying to get #inspect removed from safemode's default permitted methods due to its ability to expose instance variables.

#2 Updated by Ivan Necas over 1 year ago

  • Status changed from New to Assigned
  • Assigned To set to Ivan Necas

#3 Updated by Marek Hulán over 1 year ago

I'd suggest the rendering methods shouldn't be mixed directly into controllers and should instead be in a more isolated object, which would limit the amount of data being exposed.

That would be really awesome, one can get inspiration in remote execution plugin which implements it's own renderer . The only downside is that it would be quite big change for a security fix since it involves both TemplatesController and UnattendedController. So to fix this I'd just disable inspect globally and as a second PR we could refactor rendering.

#4 Updated by Dominic Cleal over 1 year ago

Marek Hulán wrote:

The only downside is that it would be quite big change for a security fix since it involves both TemplatesController and UnattendedController. So to fix this I'd just disable inspect globally and as a second PR we could refactor rendering.

Yes, I agree. If removing #inspect isn't possible or accepted, then we can just fix this in the next major version with a refactoring.

#5 Updated by Ivan Necas over 1 year ago

I looked into possibility to solve this in Foreman, but it's not nice at all: the problem is the inspect is allowed on the Safemode::Blankslate object
and there is not easy way to remove it form there: we would need to override the `inspect` method on the objects that are used by safemode, which
would affect their behaviour even outside of rendering.

Also, the problem is not just with the Safemode::Scope, but also with the Jail objects, where one can see attributes, that were not allowed in safemode.

Removing the inspect from the allowed methods seems like the best thing we can do right now.

#6 Updated by Ivan Necas over 1 year ago

I've opened a PR against safemode to address the issue https://github.com/svenfuchs/safemode/pull/17

#7 Updated by Dmitri Dolguikh over 1 year ago

safemode v1.2.4 that includes Ivan's fix was released today.

#8 Updated by The Foreman Bot over 1 year ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/3430 added

#9 Updated by Dominic Cleal over 1 year ago

  • Subject changed from `inspect` in a provisioning template exposes sensitive controller information to CVE-2016-3693 - `inspect` in a provisioning template exposes sensitive controller information

CVE-2016-3693 has been assigned for this issue.

#10 Updated by Dominic Cleal over 1 year ago

  • Release set to 1.11.1

#11 Updated by Ivan Necas over 1 year ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF