CVE-2015-1844 - Discovery hosts are not restricted to user taxonomies
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.
#2 Updated by Marek Hulán about 6 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...
#4 Updated by Lukas Zapletal about 6 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 about 6 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.
#9 Updated by Dominic Cleal about 6 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)?
#13 Updated by Anonymous about 6 years ago
- Status changed from Ready For Testing to Closed
- % Done changed from 0 to 100
Applied in changeset foreman_discovery|c2e61ec2c8fc00ed90588334ceedce6006db97bf.
#16 Updated by Dominic Cleal about 6 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 6 years ago
Okay let me explain this with a table. I covered the following:
|Foreman version||Discovery version||Patch applied||Vulnerable|
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.