Bug #10005
closed
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.
Updated by Marek Hulán almost 10 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...
Updated by Dominic Cleal almost 10 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.
Updated by Lukas Zapletal almost 10 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.
Updated by Marek Hulán almost 10 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.
Updated by Marek Hulán almost 10 years ago
- Related to Refactor #10025: Move taxonomy related methods and scopes to Host::Base added
Updated by Marek Hulán almost 10 years ago
Lzap you can test with https://github.com/theforeman/foreman/pull/2287
Updated by Dominic Cleal almost 10 years ago
- Related to Bug #9947: CVE-2015-1844 - GET /api/hosts doesn't respect organization/location membership added
Updated by Dominic Cleal almost 10 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)?
Updated by Dominic Cleal almost 10 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
Updated by Lukas Zapletal almost 10 years ago
The code there hasn't changed from 1.5 release, no need of backports there really.
Updated by The Foreman Bot almost 10 years ago
- Status changed from New to Ready For Testing
- Pull request https://github.com/theforeman/foreman_discovery/pull/177 added
- Pull request deleted (
)
Updated by Anonymous over 9 years ago
- Status changed from Ready For Testing to Closed
- % Done changed from 0 to 100
Applied in changeset foreman_discovery|c2e61ec2c8fc00ed90588334ceedce6006db97bf.
Updated by Dominic Cleal over 9 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?
Updated by Lukas Zapletal over 9 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.
Updated by Dominic Cleal over 9 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?
Updated by Lukas Zapletal over 9 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.
Updated by Lukas Zapletal over 9 years ago
I've verified 1.7/2.0 with patch applied, NOT vulnerable. It works, we need the core patch in 1.7.