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.

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

Add propagation string case functions and Array.join function #98

Merged
merged 15 commits into from
Mar 26, 2024

Conversation

CarlesDD
Copy link
Collaborator

@CarlesDD CarlesDD commented Mar 8, 2024

What does this PR do?

Adds propagation for string case functions toLowerCase and toUpperCase.
Adds propagation for Array.join function.

Motivation

Improve propagation.

Checklist

  • Unit tests have been updated and pass

APPSEC-47443

@CarlesDD CarlesDD requested a review from a team as a code owner March 8, 2024 08:08
src/api/array_join.cc Outdated Show resolved Hide resolved
src/api/array_join.cc Outdated Show resolved Hide resolved
int separatorLength) {
auto length = arr->Length();
int offset = 0;
auto newRanges = transaction->GetSharedVectorRange();
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are getting and potentially creating a new SharedVector each time that join method is executed, even if it doesn't have any tainted object. Can we delay the initialization of newRanges to the point that we know that it will be necessary?

}

try {
auto ranges = taintedObj ? taintedObj->getRanges() : nullptr;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The : nullptr is not possible, you're already checking !taintedObj in L57.

Suggested change
auto ranges = taintedObj ? taintedObj->getRanges() : nullptr;
auto ranges = taintedObj->getRanges();

test/js/string_case.spec.js Show resolved Hide resolved
@CarlesDD CarlesDD force-pushed the ccapell/extend-taint-tracking-operations branch from 1cbd392 to 051df64 Compare March 14, 2024 07:22
@CarlesDD CarlesDD force-pushed the ccapell/extend-taint-tracking-operations branch from 6c968b4 to c6f7d0c Compare March 22, 2024 06:45
@CarlesDD CarlesDD force-pushed the ccapell/extend-taint-tracking-operations branch from c6f7d0c to 155bbe0 Compare March 22, 2024 06:47
src/api/array_join.cc Outdated Show resolved Hide resolved
src/api/array_join.cc Show resolved Hide resolved
@CarlesDD CarlesDD merged commit 288909f into main Mar 26, 2024
57 checks passed
@CarlesDD CarlesDD deleted the ccapell/extend-taint-tracking-operations branch March 26, 2024 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants