-
Notifications
You must be signed in to change notification settings - Fork 806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restrict path segments in TenantIDs (CVE-2021-36157) #4375
Merged
pracucci
merged 1 commit into
cortexproject:master
from
simonswine:20210707_restrict-relative-segements-in-tenant-id
Jul 21, 2021
Merged
Restrict path segments in TenantIDs (CVE-2021-36157) #4375
pracucci
merged 1 commit into
cortexproject:master
from
simonswine:20210707_restrict-relative-segements-in-tenant-id
Jul 21, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This prevents paths generated from TenantIDs to become vulnerable to path traversal attacks. CVE-2021-36157 Signed-off-by: Christian Simon <simon@swine.de>
simonswine
force-pushed
the
20210707_restrict-relative-segements-in-tenant-id
branch
from
July 21, 2021 12:35
3eeff42
to
32b2d51
Compare
pracucci
approved these changes
Jul 21, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
pstibrany
approved these changes
Jul 21, 2021
bboreham
pushed a commit
that referenced
this pull request
Jul 21, 2021
This prevents paths generated from TenantIDs to become vulnerable to path traversal attacks. CVE-2021-36157 Signed-off-by: Christian Simon <simon@swine.de>
bboreham
added a commit
that referenced
this pull request
Jul 21, 2021
This prevents paths generated from TenantIDs to become vulnerable to path traversal attacks. CVE-2021-36157 Signed-off-by: Christian Simon <simon@swine.de>
Closed
alvinlin123
pushed a commit
to ac1214/cortex
that referenced
this pull request
Jan 14, 2022
This prevents paths generated from TenantIDs to become vulnerable to path traversal attacks. CVE-2021-36157 Signed-off-by: Christian Simon <simon@swine.de> Signed-off-by: Alvin Lin <alvinlin@amazon.com>
alvinlin123
pushed a commit
to ac1214/cortex
that referenced
this pull request
Jan 14, 2022
…ct#4376) This prevents paths generated from TenantIDs to become vulnerable to path traversal attacks. CVE-2021-36157 Signed-off-by: Christian Simon <simon@swine.de> Signed-off-by: Alvin Lin <alvinlin@amazon.com>
simonswine
added a commit
to simonswine/dskit
that referenced
this pull request
Feb 21, 2022
This prevents paths generated from TenantIDs to become vulnerable to path traversal attacks. CVE-2021-36157 Signed-off-by: Christian Simon <simon@swine.de>
simonswine
pushed a commit
to simonswine/dskit
that referenced
this pull request
Feb 21, 2022
…exproject/cortex#4376) This prevents paths generated from TenantIDs to become vulnerable to path traversal attacks. CVE-2021-36157 Signed-off-by: Christian Simon <simon@swine.de>
simonswine
pushed a commit
to grafana/dskit
that referenced
this pull request
Mar 22, 2022
…exproject/cortex#4376) This prevents paths generated from TenantIDs to become vulnerable to path traversal attacks. CVE-2021-36157 Signed-off-by: Christian Simon <simon@swine.de>
simonswine
added a commit
to grafana/dskit
that referenced
this pull request
Mar 29, 2022
* Add tenant resolver (cortexproject/cortex#3486) * Add tenant resolver package This implements the multi tenant resolver as described by the [proposal] for multi tenant query-federation. By default it behaves like before, but it's implementation can be swapped out. [proposal]: cortexproject/cortex#3364 Signed-off-by: Christian Simon <simon@swine.de> * Replace usages of `ExtractOrgID` Use TenantID or UserID depending on which of the methods are meant to be used. Signed-off-by: Christian Simon <simon@swine.de> * Replace usages of `ExtractOrgIDFromHTTPRequest` This is replaced by ExtractTenantIDFromHTTPRequest, which makes sure that exactly one tenant ID is set. Signed-off-by: Christian Simon <simon@swine.de> * Add methods to `tenant` package to use resolver directly Signed-off-by: Christian Simon <simon@swine.de> * Remove UserID method from Resolver interface We need a better definition for what we are trying to achieve with UserID before we can add it to the interface Signed-off-by: Christian Simon <simon@swine.de> * Update comment on the TenantID/TenantIDs Signed-off-by: Christian Simon <simon@swine.de> * Improve performance of NormalizeTenantIDs - reduce allocations by reusing the input slice during de-duplication Signed-off-by: Christian Simon <simon@swine.de> * Add multi tenant query federation (cortexproject/cortex#3250) * Add tenant query federation This experimental feature allows queries to cover data from more than a single Cortex tenant and can be activated by supplying `-tenant-federation.enabled` to all cortex instances. To query multiple tenants a `|` separated tenant list can be specified in the `X-Scope-OrgID` header. The source tenant of a metric will be exposed in the label `__tenant_id__`. Signed-off-by: Christian Simon <simon@swine.de> * Aggregate the limit of maxQueriers correctly This ensures the limit is aggregated correctly of the setting of each individual tenant. It also implements the logic for the v2 query frontend. Signed-off-by: Christian Simon <simon@swine.de> * Fix tenant labels and make LabelNames more efficient Signed-off-by: Christian Simon <simon@swine.de> * Use tsdb_errors for error handling Signed-off-by: Christian Simon <simon@swine.de> * Fix uninitialized label matcher Regexp matcher need to be initialized, this adds a test for regexp matcher and fixes the underlying issue. Signed-off-by: Christian Simon <simon@swine.de> * Use map for filterValuesByMatchers to reduce allocations Signed-off-by: Christian Simon <simon@swine.de> * Address review suggestions Signed-off-by: Christian Simon <simon@swine.de> * Add validation.SmallestPositiveNonZeroIntPerTenant to avoid code duplication Signed-off-by: Christian Simon <simon@swine.de> * Add limitations and experimental status to docs Signed-off-by: Christian Simon <simon@swine.de> * Remove unnecessary cast of defaultTenantLabel Signed-off-by: Christian Simon <simon@swine.de> * Handle query-range limits for multi tenant queries Signed-off-by: Christian Simon <simon@swine.de> * Skip results cache, if multi tenants query Signed-off-by: Christian Simon <simon@swine.de> * Add failint to ensure query path supports multiple tenants To avoid any future regressions in the multi tenant support within the query path, a faillint command tests if TenantID is used and if it finds one, it suggestest using TenantIDs instead> Signed-off-by: Christian Simon <simon@swine.de> * Align CHANGELOG line with the flag description Signed-off-by: Christian Simon <simon@swine.de> * Add a limitation about missing results cache Signed-off-by: Christian Simon <simon@swine.de> * Restrict path segments in TenantIDs (cortexproject/cortex#4375) (cortexproject/cortex#4376) This prevents paths generated from TenantIDs to become vulnerable to path traversal attacks. CVE-2021-36157 Signed-off-by: Christian Simon <simon@swine.de> * Update nolint statement Co-authored-by: Bryan Boreham <bjboreham@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does:
This prevents paths derived from TenantIDs to become vulnerable to path traversal attacks. CVE-2021-36157
Edit added more details.
An attacker, with suitable access, could trick Cortex into sending the contents of files it has access to.
The vulnerability is that the header value
X-Scope-OrgID
is used to construct file paths for rules files, and if crafted to say something like../../sensitive/path/in/deployment
then Cortex will attempt to parse a rules file at that location and include some of the contents in the error message.Other Cortex API requests can also be sent a malicious OrgID header, e.g. tricking the ingester into writing metrics to a different location, but the effect is nuisance rather than disclosure.
The severity is rated as Medium
Mitigations:
If you have a proxy in front of Cortex that supplies the OrgID header, so it cannot be crafted by an attacker, then you are not vulnerable. We always recommend such a proxy - https://cortexmetrics.io/docs/guides/auth/
If you run Cortex with limited access to sensitive files, e.g. in a container or chroot, and as a user with limited access, then this will constrain the vulnerability to that access.