Bug #10005
CVE-2015-1844 - Discovery hosts are not restricted to user taxonomies
Description
I found security issue that's very similar to what is being fixed in Foreman and tracked as http://projects.theforeman.org/issues/9947 The Foreman fix doesn't fix it, because it's present in discovery codebase. See https://github.com/theforeman/foreman_discovery/blob/develop/app/models/host/discovered.rb#L30
This allows user to manipulate all discovered hosts (if they have global permission) even if they are assigned to specific org/loc. The fix is either to apply the same patch as we did in Foreman Host::Managed or (better) move the fix to Host::Base and remove this default scope from discovery.
Related issues
Associated revisions
History
#2
Updated by Marek Hulán almost 8 years ago
Yes, core bug is public. I'd wait with public PR until confirmed by others. I don't see a point why to duplicate this default_scope. Host::Base already has notion of taxonomies https://github.com/theforeman/foreman/blob/develop/app/models/host/base.rb#L202. Of course belongs_to $taxonomy should be moved to Base as well...
#3
Updated by Dominic Cleal almost 8 years ago
Small reminder - please don't mark notes as private within a private bug, all will be private automatically. It also means when the bug is made public, discussion will be visible.
#4
Updated by Lukas Zapletal almost 8 years ago
From comment 1: "I am against moving it to Host::Base that could be quite confusing. I will do the same patch in Host::Discovered."
Since Host::Base already contains that, I agree this would be good fit. Can you refactor this please? I will test this with Discovery plugin.
#5
Updated by Marek Hulán almost 8 years ago
Btw even if we move the taxonomy related stuff into Host::Base, you should probably prepare a patch that fixes the default scope in discovery for older versions. The move to Host::Base won't be backported. And after the move you still have to remove default_scope so it does not redefine the one from Base.
#6
Updated by Marek Hulán almost 8 years ago
- Related to Refactor #10025: Move taxonomy related methods and scopes to Host::Base added
#7
Updated by Marek Hulán almost 8 years ago
Lzap you can test with https://github.com/theforeman/foreman/pull/2287
#8
Updated by Dominic Cleal almost 8 years ago
- Related to Bug #9947: CVE-2015-1844 - GET /api/hosts doesn't respect organization/location membership added
#9
Updated by Dominic Cleal almost 8 years ago
- Private changed from Yes to No
This is being handled as a continuation of CVE-2015-1844, specifically targeting Foreman Discovery.
#10025 has been merged to develop and 1.8-stable now, and I've marked it for backport to 1.7.5. I have just noticed comment 5 which says it won't be backported - do we need a different fix for Discovery 2.x? Any changes required for 3.x (for 1.8/1.9)?
#10
Updated by Dominic Cleal almost 8 years ago
- Subject changed from Discovery hosts are not restricted to user taxonomies to CVE-2015-1844 - Discovery hosts are not restricted to user taxonomies
#11
Updated by Lukas Zapletal almost 8 years ago
The code there hasn't changed from 1.5 release, no need of backports there really.
#12
Updated by The Foreman Bot almost 8 years ago
- Status changed from New to Ready For Testing
- Pull request https://github.com/theforeman/foreman_discovery/pull/177 added
- Pull request deleted (
)
#13
Updated by Anonymous almost 8 years ago
- Status changed from Ready For Testing to Closed
- % Done changed from 0 to 100
Applied in changeset foreman_discovery|c2e61ec2c8fc00ed90588334ceedce6006db97bf.
#14
Updated by Dominic Cleal almost 8 years ago
Lukas Zapletal wrote:
The code there hasn't changed from 1.5 release, no need of backports there really.
If it hasn't changed, then perhaps we do need to backport it to 2.x?
#15
Updated by Lukas Zapletal almost 8 years ago
Dominic Cleal wrote:
Lukas Zapletal wrote:
The code there hasn't changed from 1.5 release, no need of backports there really.
If it hasn't changed, then perhaps we do need to backport it to 2.x?
I have tested discovery 2.0 and it is not affected.
#16
Updated by Dominic Cleal almost 8 years ago
Lukas Zapletal wrote:
Dominic Cleal wrote:
Lukas Zapletal wrote:
The code there hasn't changed from 1.5 release, no need of backports there really.
If it hasn't changed, then perhaps we do need to backport it to 2.x?
I have tested discovery 2.0 and it is not affected.
Ok, was that with the #10025 patch on 1.7-stable or without?
I know 3.x has gained some taxonomy features, but did they introduce an issue here somehow?
#17
Updated by Lukas Zapletal almost 8 years ago
Okay let me explain this with a table. I covered the following:
Foreman version | Discovery version | Patch applied | Vulnerable |
1.7 | 2.0 | No | Yes |
1.7 | 2.0 | Yes | Not tested |
1.8 | 3.0 | No | Yes |
1.8 | 3.0 | Yes | No |
Since the bug #9947 is scheduled for 1.7 and since there were no changes in regard to taxonomy between 2.0 and 3.0 versions, I don't believe it's vulnerable. HAVEN'T TESTED THO. The patch does not apply cleanly.
#18
Updated by Lukas Zapletal almost 8 years ago
I've verified 1.7/2.0 with patch applied, NOT vulnerable. It works, we need the core patch in 1.7.
Fixes #10005 - removed unused default scope and added tax tests