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

CSP issue with inline scripts on checkout, after applying the 2.4.6-p6 patch #87

Open
redo-interactive opened this issue Jun 19, 2024 · 4 comments

Comments

@redo-interactive
Copy link

redo-interactive commented Jun 19, 2024

After applying latest security patch from Adobe Commerce/Magento there is an issue on checkout, with inline script in

view/frontend/templates/js.phtml

There is a need to use
$secureRenderer->renderTag to generate script with unique nonce.

Magento version #:

2.4.7-p1, 2.4.6-p6, 2.4.5-p8, 2.4.4-p9

Edition (EE, CE, OS, etc):

EE, CE, OS

Expected behavior:

js scripts won't break execution on checkout.

Actual behavior:

js scripts are breaking execution on checkout.

Steps to reproduce:

add product to the cart and go to checkout

Preconditions

M2/AC 2.4.6-p6, PHP 8.1

I have created a fix for this:

<?php
/**
 * Copyright © MagePal LLC. All rights reserved.
 * See COPYING.txt for license details.
 * http://www.magepal.com | support@magepal.com
 */

/** @var $block MagePal\GoogleTagManager\Block\DataLayer **/
$dataLayerName = $escaper->escapeJs($block->getDataLayerName());
$accountId = $escaper->escapeJs($block->getAccountId());
$containerCode = $block->getEmbeddedCode() ? "+'{$block->getEmbeddedCode()}'" : '';

$gtmScript = <<<SCRIPT
    window.{$dataLayerName} = window.{$dataLayerName} || [];
    SCRIPT;

if (!$block->isGdprEnabled() && $block->addJsInHead() && !$block->isAdvancedSettingsEnabled()) {
    $dataLayerJs = $block->getDataLayerJs();
    $gtmScript .= <<<SCRIPT
            {$dataLayerJs}
            (function(w,d,s,l,i){
            w[l]=w[l]||[];
            w[l].push({'gtm.start': new Date().getTime(),event:'gtm.js'});
            var f=d.getElementsByTagName(s)[0],
                j=d.createElement(s),
                dl=l!='{$dataLayerName}'?'&l='+l:'';
            j.async=true;
            j.src='https://www.googletagmanager.com/gtm.js?id='+i+dl'{$containerCode}';
            f.parentNode.insertBefore(j,f);
        })(window,document,'script','{$dataLayerName}','{$accountId}');
        SCRIPT;
}

if ($block->isAdvancedSettingsEnabled()) {
    $advancedSettingsJsCode = $block->getAdvancedSettingsJsCode();
    $gtmScript .= <<<SCRIPT
            {$dataLayerJs}
            {$advancedSettingsJsCode}
            SCRIPT;
}

// phpcs:ignore
echo /* @noEscape */ $secureRenderer->renderTag('script', [], $gtmScript, false); ?>

<?php if (($block->isGdprEnabled() || !$block->addJsInHead()) && !$block->isAdvancedSettingsEnabled()): ?>
    <script type="text/x-magento-init">
        {
            "*": {
                "magepalGtmDatalayer": {
                    "isCookieRestrictionModeEnabled": <?= /* @noEscape */ $block->isCookieRestrictionModeEnabled() ?>,
                    "currentWebsite": <?= /* @noEscape */ $block->getCurrentWebsiteId() ?>,
                    "cookieName": "<?= /* @noEscape */ $block->getCookieRestrictionName() ?>",
                    "dataLayer": "<?= /* @noEscape */ $block->getDataLayerName() ?>",
                    "accountId": "<?= /* @noEscape */ $block->getAccountId() ?>",
                    "data": <?= /* @noEscape */ $block->getDataLayerJson() ?>,
                    "isGdprEnabled": <?= /* @noEscape */ $block->isGdprEnabled() ?>,
                    "gdprOption": <?= /* @noEscape */ $block->getGdprOption() ?>,
                    "addJsInHeader": <?= /* @noEscape */ $block->addJsInHead() ?>,
                    "containerCode": "<?= /* @noEscape */ $block->getEmbeddedCode() ?>"
                }
            }
        }
    </script>
<?php else : ?>
    <script type="text/x-magento-init">
        {
            "*": {
                "magepalGtmDatalayer": {
                    "dataLayer": "<?= /* @noEscape */ $block->getDataLayerName() ?>"
            }
        }
    }
    </script>
<?php endif; ?>
<!-- End Google Tag Manager by MagePal -->
@redo-interactive
Copy link
Author

redo-interactive commented Jun 20, 2024

the patch for current version of this file:

diff --git a/view/frontend/templates/js.phtml b/view/frontend/templates/js.phtml
index b6d42fe..c37a269 100755
--- a/view/frontend/templates/js.phtml
+++ b/view/frontend/templates/js.phtml
@@ -6,60 +6,70 @@
  */
 
 /** @var $block MagePal\GoogleTagManager\Block\DataLayer **/
-$dataLayerName = $block->getDataLayerName();
-$accountId = $block->getAccountId();
-$containerCode = $block->getEmbeddedCode();
-?>
+$dataLayerName = $escaper->escapeJs($block->getDataLayerName());
+$accountId = $escaper->escapeJs($block->getAccountId());
+$containerCode = $block->getEmbeddedCode() ? "+'{$block->getEmbeddedCode()}'" : '';
 
-<!-- Google Tag Manager by MagePal -->
-<script type="text/javascript">
-    window.<?= /* @noEscape */ $dataLayerName ?> = window.<?= /* @noEscape */ $dataLayerName ?> || [];
+$gtmScript = <<<SCRIPT
+    window.{$dataLayerName} = window.{$dataLayerName} || [];
+    SCRIPT;
 
-<?php if (!$block->isGdprEnabled() && $block->addJsInHead() && !$block->isAdvancedSettingsEnabled()): ?>
-    <?= /* @noEscape */ $block->getDataLayerJs() ?>
-    (function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start':
-            new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0],
-        j=d.createElement(s),dl=l!='<?= /* @noEscape */ $dataLayerName ?>'?'&l='+l:'';j.async=true;j.src=
-        'https://www.googletagmanager.com/gtm.js?id='+i+dl<?= /* @noEscape */ $containerCode ? "+'{$containerCode}'" : '' ?>;f.parentNode.insertBefore(j,f);
-    })(window,document,'script','<?= /* @noEscape */ $dataLayerName ?>','<?= /* @noEscape */ $accountId ?>');
-<?php endif; ?>
-</script>
+if (!$block->isGdprEnabled() && $block->addJsInHead() && !$block->isAdvancedSettingsEnabled()) {
+    $dataLayerJs = $block->getDataLayerJs();
+    $gtmScript .= <<<SCRIPT
+            {$dataLayerJs}
+            (function(w,d,s,l,i){
+            w[l]=w[l]||[];
+            w[l].push({'gtm.start': new Date().getTime(),event:'gtm.js'});
+            var f=d.getElementsByTagName(s)[0],
+                j=d.createElement(s),
+                dl=l!='{$dataLayerName}'?'&l='+l:'';
+            j.async=true;
+            j.src='https://www.googletagmanager.com/gtm.js?id='+i+dl'{$containerCode}';
+            f.parentNode.insertBefore(j,f);
+        })(window,document,'script','{$dataLayerName}','{$accountId}');
+        SCRIPT;
+}
 
-<?php if ($block->isAdvancedSettingsEnabled()): ?>
-<script type="text/javascript">
-    <?= /* @noEscape */ $block->getDataLayerJs() ?>
-</script>
-    <?= /* @noEscape */ $block->getAdvancedSettingsJsCode() ?>
-<?php endif; ?>
+if ($block->isAdvancedSettingsEnabled()) {
+    $advancedSettingsJsCode = $block->getAdvancedSettingsJsCode();
+    $gtmScript .= <<<SCRIPT
+            {$dataLayerJs}
+            {$advancedSettingsJsCode}
+            SCRIPT;
+}
 
-<?php if (($block->isGdprEnabled() || !$block->addJsInHead()) && !$block->isAdvancedSettingsEnabled()) : ?>
-<script type="text/x-magento-init">
-    {
-        "*": {
-            "magepalGtmDatalayer": {
-                "isCookieRestrictionModeEnabled": <?= /* @noEscape */ $block->isCookieRestrictionModeEnabled() ?>,
-                "currentWebsite": <?= /* @noEscape */ $block->getCurrentWebsiteId() ?>,
-                "cookieName": "<?= /* @noEscape */ $block->getCookieRestrictionName() ?>",
-                "dataLayer": "<?= /* @noEscape */ $block->getDataLayerName() ?>",
-                "accountId": "<?= /* @noEscape */ $block->getAccountId() ?>",
-                "data": <?= /* @noEscape */ $block->getDataLayerJson() ?>,
-                "isGdprEnabled": <?= /* @noEscape */ $block->isGdprEnabled() ?>,
-                "gdprOption": <?= /* @noEscape */ $block->getGdprOption() ?>,
-                "addJsInHeader": <?= /* @noEscape */ $block->addJsInHead() ?>,
-                "containerCode": "<?= /* @noEscape */ $block->getEmbeddedCode() ?>"
+// phpcs:ignore
+echo /* @noEscape */ $secureRenderer->renderTag('script', [], $gtmScript, false); ?>
+
+<?php if (($block->isGdprEnabled() || !$block->addJsInHead()) && !$block->isAdvancedSettingsEnabled()): ?>
+    <script type="text/x-magento-init">
+        {
+            "*": {
+                "magepalGtmDatalayer": {
+                    "isCookieRestrictionModeEnabled": <?= /* @noEscape */ $block->isCookieRestrictionModeEnabled() ?>,
+                    "currentWebsite": <?= /* @noEscape */ $block->getCurrentWebsiteId() ?>,
+                    "cookieName": "<?= /* @noEscape */ $block->getCookieRestrictionName() ?>",
+                    "dataLayer": "<?= /* @noEscape */ $block->getDataLayerName() ?>",
+                    "accountId": "<?= /* @noEscape */ $block->getAccountId() ?>",
+                    "data": <?= /* @noEscape */ $block->getDataLayerJson() ?>,
+                    "isGdprEnabled": <?= /* @noEscape */ $block->isGdprEnabled() ?>,
+                    "gdprOption": <?= /* @noEscape */ $block->getGdprOption() ?>,
+                    "addJsInHeader": <?= /* @noEscape */ $block->addJsInHead() ?>,
+                    "containerCode": "<?= /* @noEscape */ $block->getEmbeddedCode() ?>"
+                }
             }
         }
-    }
-</script>
+    </script>
 <?php else : ?>
-<script type="text/x-magento-init">
-    {
-        "*": {
-            "magepalGtmDatalayer": {
-                "dataLayer": "<?= /* @noEscape */ $block->getDataLayerName() ?>"
+    <script type="text/x-magento-init">
+        {
+            "*": {
+                "magepalGtmDatalayer": {
+                    "dataLayer": "<?= /* @noEscape */ $block->getDataLayerName() ?>"
             }
         }
     }
-</script>
+    </script>
 <?php endif; ?>
 <!-- End Google Tag Manager by MagePal -->


@srenon
Copy link
Contributor

srenon commented Jun 20, 2024

@redo-interactive ... thank for the code, we already have all the coding done internally and will be releasing shortly after a full review since we will need to drop support for older version of Magento < 2.4.0

They seem to be a few issues and bug right here in your code
image

You may have an issue when this getEmbeddedCode() is null

$containerCode = $block->getEmbeddedCode() ? "+'{$block->getEmbeddedCode()}'" : '';

@redo-interactive
Copy link
Author

redo-interactive commented Jun 20, 2024

@srenon Oh you are right. Thank you.

just updated the code.

@norgeindian
Copy link

@srenon , could you share some details, when the new version will be available?

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

No branches or pull requests

3 participants