Skip to content

[Backport] Use constant time string comparison in FormKey validator#17108

Merged
magento-engcom-team merged 3 commits intomagento:2.1-developfrom
gelanivishal:2.1-develop-PR-port-13509
Aug 6, 2018
Merged

[Backport] Use constant time string comparison in FormKey validator#17108
magento-engcom-team merged 3 commits intomagento:2.1-developfrom
gelanivishal:2.1-develop-PR-port-13509

Conversation

@gelanivishal
Copy link
Copy Markdown
Contributor

@gelanivishal gelanivishal commented Jul 25, 2018

Original Pull Request

#13509

Description

CSRF tokens should be considered sensitive strings. While the risk of a malicious actor attempting gleam the form key via a timing attack is very low, we should still follow best practices in verifying this token.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@VladimirZaets
Copy link
Copy Markdown
Contributor

Hi @gelanivishal. Thanks for the contribution. We can't make backport of @api annotation to Magento 2.1 version because we can't have API annotation with intervals. After API annotation was declared all next versions should have the declared annotation, but in the current case, Magento 2.1 version will contain the annotation but Magento 2.2.0 will not.

@gelanivishal
Copy link
Copy Markdown
Contributor Author

@VladimirZaets I have removed API annotation and let me know anything you want to change.

Thank you.

@gelanivishal
Copy link
Copy Markdown
Contributor Author

@VladimirZaets Have you checked requested changes?

@sidolov sidolov self-assigned this Aug 5, 2018
@sidolov sidolov self-requested a review August 5, 2018 11:08
@magento-engcom-team
Copy link
Copy Markdown
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-2656 has been created to process this Pull Request

@magento-engcom-team
Copy link
Copy Markdown
Contributor

Hi @gelanivishal. Thank you for your contribution.
We will aim to release these changes as part of 2.1.15.
Please check the release notes for final confirmation.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants