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

Templates XSS Security and short echo tag syntax #99

Closed
noire-munich opened this issue May 21, 2019 · 1 comment
Closed

Templates XSS Security and short echo tag syntax #99

noire-munich opened this issue May 21, 2019 · 1 comment
Labels
enhancement Improvements to existing rules

Comments

@noire-munich
Copy link

Description

Consider the following code:
<input type="hidden" name="<?php /* @escapeNotVerified */ echo $block->getInputElementName();?>" value="" id="<?php /* @escapeNotVerified */ echo $_id;?>"

A template with this snippet will raise the following warning:

x | WARNING | Short echo tag syntax must be used; expected "<?=" but found "<?php echo"

Expected behavior

Template XSS Security tags are here to improve the quality of code and so are EQP tests. It seems of higher value to defend XSS Security tags than to promote the use of short echo tags over it. We should not see a Warning when a XSS Security Annotation is used.

Benefits

EQP Standards would reflect better Magento's recommandations & tools.
Reports would be less bloated with Warnings with little to no value.

Additional information

@noire-munich noire-munich added the enhancement Improvements to existing rules label May 21, 2019
@noire-munich noire-munich reopened this May 21, 2019
@lenaorobei
Copy link
Contributor

Hello @foggyreads,

The code snipped you've posted will raise following:

-----------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 1 LINE
-----------------------------------------------------------------------------------------
 1 | WARNING | Short echo tag syntax must be used; expected "<?=" but found "<?php echo"
 1 | WARNING | Unescaped output detected.
 1 | WARNING | Short echo tag syntax must be used; expected "<?=" but found "<?php echo"
 1 | WARNING | Unescaped output detected.
 1 | WARNING | Line exceeds maximum limit of 120 characters; contains 154 characters
-----------------------------------------------------------------------------------------

The sniff which detects Short echo tag syntax misusage is Magento2.PHP.ShortEchoSyntax. It is all about code style, not about XSS. Please see related Magento 2 PR magento/magento2#1563

For XSS purpose Magento2.Security.XssTemplate was implemented and it covers unescaped output.

Hope it makes sense.
Feel free to reopen this issue if you have better proposal of how to handle it.
Thanks.

magento-devops-reposync-svc pushed a commit that referenced this issue Oct 14, 2021
…oding-standard-274

[Imported] AC-663 PHPCS classes test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing rules
Projects
None yet
Development

No branches or pull requests

2 participants