Skip to content

Conversation

@aschackmull
Copy link
Contributor

The few additional results (https://lgtm.com/query/1236408154146688854/) seem like an improvement.

@aschackmull aschackmull requested a review from a team as a code owner August 12, 2020 06:27
@adityasharad adityasharad changed the base branch from master to main August 14, 2020 18:33
@aibaars
Copy link
Contributor

aibaars commented Aug 31, 2020

Looks fine to me, although I don't really like the increase in false positives. We now flag any remote user input that flows into the command array, while really the most dangerous is the first element in the array or potentially the others if the first is some sort of shell or interpreter (java, python, etc). I suppose we can refine the query later if the false positive rate is considered too high.

@aschackmull
Copy link
Contributor Author

Superseded by #4287

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants