GreenMeteor / codebox

Allows you to add and use HTML snippets on your sidebar
3 stars 5 forks source link

Bug: Nonce not applied #5

Closed luke- closed 8 months ago

luke- commented 8 months ago

Reproduce:

  1. Create a inline script with with nonce e.g. <script nonce={{nonce}}>alert(1)</script>
  2. The nonce is not applied correctly.

In the source I can see the nonce variable is passed to the view here: https://github.com/GreenMeteor/codebox/blob/master/widgets/CodeboxFrame.php#L34

But on the view side it's not used: https://github.com/GreenMeteor/codebox/blob/master/widgets/views/codeboxframe.php

ArchBlood commented 8 months ago

Reproduce:

  1. Create a inline script with with nonce e.g. <script nonce={{nonce}}>alert(1)</script>
  2. The nonce is not applied correctly.

In the source I can see the nonce variable is passed to the view here: https://github.com/GreenMeteor/codebox/blob/master/widgets/CodeboxFrame.php#L34

But on the view side it's not used: https://github.com/GreenMeteor/codebox/blob/master/widgets/views/codeboxframe.php

You are correct, I've tested and see that it doesn't pass the value of nonce even though it is set, I'll have to look into this behavior more to see why. Originally it used the same method as the Html Statistics page which works fine.

luke- commented 8 months ago

Here is how the Statistic Code is working: https://github.com/humhub/humhub/blob/master/protected/humhub/modules/admin/widgets/TrackingWidget.php#L38-L40

ArchBlood commented 8 months ago

Here is how the Statistic Code is working: https://github.com/humhub/humhub/blob/master/protected/humhub/modules/admin/widgets/TrackingWidget.php#L38-L40

That's where it seems the two are different, one is using the template engine and the other isn't, I've also tested with this and the issue changes but forces the template engine on the developer and loses the original panels and containers for sidebars, the nonce is present just the end part is hidden which is indicated by the console error so either way it does the same thing just changes the layout display;

dashboard:523 Refused to execute inline script because it violates the following Content Security Policy directive: "script-src 'nonce-y9Ix3oNqrtSDT+tyILHhi/tn' 'self' https://* http://* * 'unsafe-inline' 'report-sample'". Note that 'unsafe-inline' is ignored if either a hash or nonce value is present in the source list.

I've tested with Html::nonce() which does the same thing as well, I'll have to find a better solution for this.

luke- commented 8 months ago

Something like this?

$htmlCode = str_replace('nonce={{nonce}}', Html::nonce(), $htmlCode);

Here: https://github.com/GreenMeteor/codebox/blob/master/widgets/CodeboxFrame.php#L33

ArchBlood commented 8 months ago

Something like this?

$htmlCode = str_replace('nonce={{nonce}}', Html::nonce(), $htmlCode);

Here: https://github.com/GreenMeteor/codebox/blob/master/widgets/CodeboxFrame.php#L33

This would be a good approach if it weren't for $htmlCode being equal to Yii::$app->getModule('codebox')->getHtmlCode();, I think the better approach would be in the following lines of the Module.php;

https://github.com/GreenMeteor/codebox/blob/e0ff3aa8a79d400cbe5963b6577fb4cf8d517887/Module.php#L31-L38

luke- commented 8 months ago

that would also be ok, as long as you replace the current nonce.

ArchBlood commented 8 months ago

that would also be ok, as long as you replace the current nonce.

It seems like #6 fixes the console errors when it comes to nonce, but it still isn't showing the full nonce, so far this is only partly fixed.

ArchBlood commented 8 months ago

Okay, now it should work correctly, @luke- can you review the code to make sure it works on your end?