Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ML] Various fixes for possible prototype pollution vulnerabilities #194529

Merged
merged 4 commits into from
Oct 2, 2024
Merged

[ML] Various fixes for possible prototype pollution vulnerabilities #194529

merged 4 commits into from
Oct 2, 2024

Conversation

jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Oct 1, 2024 •
edited by kibanamachine
Loading

Fixes potential prototype pollution vulnerability in setNestedProperty function.
Fixes incomplete string escaping issue in ML's saved object service.

@jgowdyelastic jgowdyelastic self-assigned this Oct 1, 2024
@jgowdyelastic jgowdyelastic changed the title [ML] Various fixes for code scanning alert issues [ML] Various fixes for possible prototype pollution vulnerabilites Oct 1, 2024
@jgowdyelastic jgowdyelastic changed the title [ML] Various fixes for possible prototype pollution vulnerabilites [ML] Various fixes for possible prototype pollution vulnerabilities Oct 1, 2024
@jgowdyelastic jgowdyelastic marked this pull request as ready for review October 1, 2024 11:14
@jgowdyelastic jgowdyelastic requested a review from a team as a code owner October 1, 2024 11:14
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

peteharverson
peteharverson approved these changes Oct 1, 2024
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@peteharverson peteharverson mentioned this pull request Oct 1, 2024
Closed
23 tasks
walterra
walterra reviewed Oct 1, 2024
View reviewed changes
x-pack/packages/ml/nested_property/src/set_nested_property.ts Outdated
@@ -7,7 +7,7 @@

export const setNestedProperty = (obj: Record<string, any>, accessor: string, value: any) => {
let ref = obj;
const accessors = accessor.split('.');
const accessors = accessor.split('.').filter((a) => a !== '__proto__');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider constructor too? I'm unsure if we should silently ignore like now or do the check in the for loop and throw, what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 734ae86

walterra
walterra approved these changes Oct 1, 2024
View reviewed changes
Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jgowdyelastic
Copy link
Member Author

@walterra I've updated the check to throw an error now as per our offline discussion. Could you please take another look.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #14 / Timeline rendering should trim kqlQueryExpression

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
transform 585.2KB 585.3KB +116.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jgowdyelastic

@jgowdyelastic jgowdyelastic merged commit d1f24b0 into elastic:main Oct 2, 2024
25 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11139856866

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 2, 2024
@jgowdyelastic
[ML] Various fixes for possible prototype pollution vulnerabilities (e…
35f7c7c 
…lastic#194529)

Fixes potential prototype pollution vulnerability in `setNestedProperty`
function.
Fixes incomplete string escaping issue in ML's saved object service.

(cherry picked from commit d1f24b0)
@kibanamachine kibanamachine mentioned this pull request Oct 2, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

This was referenced Oct 2, 2024
Merged
Merged
kibanamachine added a commit that referenced this pull request Oct 2, 2024
@kibanamachine @jgowdyelastic
[8.x] [ML] Various fixes for possible prototype pollution vulnerabili…
28e012b 
…ties (#194529) (#194660)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ML] Various fixes for possible prototype pollution vulnerabilities
(#194529)](#194529)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"James
Gowdy","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-02T07:47:19Z","message":"[ML]
Various fixes for possible prototype pollution vulnerabilities
(#194529)\n\nFixes potential prototype pollution vulnerability in
`setNestedProperty`\r\nfunction.\r\nFixes incomplete string escaping
issue in ML's saved object
service.","sha":"d1f24b050b53cc7b13fbc47b6de3c5f69606e88e","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix",":ml","v9.0.0","v8.16.0","backport:version"],"title":"[ML]
Various fixes for possible prototype pollution
vulnerabilities","number":194529,"url":"https://github.com/elastic/kibana/pull/194529","mergeCommit":{"message":"[ML]
Various fixes for possible prototype pollution vulnerabilities
(#194529)\n\nFixes potential prototype pollution vulnerability in
`setNestedProperty`\r\nfunction.\r\nFixes incomplete string escaping
issue in ML's saved object
service.","sha":"d1f24b050b53cc7b13fbc47b6de3c5f69606e88e"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194529","number":194529,"mergeCommit":{"message":"[ML]
Various fixes for possible prototype pollution vulnerabilities
(#194529)\n\nFixes potential prototype pollution vulnerability in
`setNestedProperty`\r\nfunction.\r\nFixes incomplete string escaping
issue in ML's saved object
service.","sha":"d1f24b050b53cc7b13fbc47b6de3c5f69606e88e"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: James Gowdy <[email protected]>
@peteharverson peteharverson added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:fix labels Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peteharverson peteharverson peteharverson approved these changes

@walterra walterra walterra approved these changes

Assignees

@jgowdyelastic jgowdyelastic

Labels
backport:version Backport to applied version labels :ml release_note:skip Skip the PR/issue when compiling release notes v8.16.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jgowdyelastic @elasticmachine @kibana-ci @kibanamachine @walterra @peteharverson