fix Type confusion through parameter tampering #3293
Closed
kreeksec wants to merge 1 commit intoarkime:mainfrom
kreeksec:patch-1
Closed
fix Type confusion through parameter tampering #3293kreeksec wants to merge 1 commit intoarkime:mainfrom kreeksec:patch-1
kreeksec wants to merge 1 commit intoarkime:mainfrom
kreeksec:patch-1
Conversation
Contributor
|
We already call ArkimeUtil.isString above, plus everything in req.query is already a string. Did you find an issue or is this just AI? |
Contributor
|
Closing for now, fell free to reopen with more details |
This file contains hidden or 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
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.
arkime/viewer/apiConnections.js
Line 90 in 10d1dca
Sanitizing untrusted HTTP request parameters is a common technique for preventing injection attacks such as SQL injection or path traversal. This is sometimes done by checking if the request parameters contain blacklisted substrings. However, sanitizing request parameters assuming they have type
Stringand using the builtin string methods such asString.prototype.indexOfis susceptible to type confusion attacks. In a type confusion attack, an attacker tampers with an HTTP request parameter such that it has a value of typeArrayinstead of the expected typeString. Furthermore, the content of the array has been crafted to bypass sanitizers by exploiting that some identically named methods of strings and arrays behave differently.Fix the problem, we need to ensure that
req.query.baselineDateis a string before calling any string methods such as.endsWithor.slice. The best way is to add a type check (usingtypeofor a robust utility) before using these methods. If the value is not a string, we should handle it as an error or sanitize it appropriately. The fix should be applied in the block wherebaselineDateTmpStris set and used (lines 87–91). We should also ensure that the early return on line 80 is triggered if the type is not a string, to prevent further processing of invalid input. This can be done by moving the type check closer to where the value is used, or by adding an explicit check before using string methods.References
Node.js API querystring