Add SMW Immersion theme plugin for OJS 3.5.0 compatibility

[Godoy0722]

No description provided.

[Godoy0722]

Hello @bozana , would you mind having a look on this plugin? This will be a new theme in our plugin gallery. Also, would you mind having a look on the theme itself to make a code review on that? I appreciate your help!

[bozana]

Hi @Godoy0722,

There are a few security issues:

• Raw HTML galley injected via innerHTML in the JS code in templates/plugins/generic/htmlArticleGalley/templates/display.tpl. I am not sure if maybe DOMPurify.sanitize(articleBody) could be used.
• The smwTOC() concatenates DOM values into an HTML string instead of maybe using DOM APIs. The same with references (lines 480–484), copyright-info (lines 524–527)
• Not correct or no escaping of variables in Smarty templates, e.g.
  https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/templates/frontend/objects/article_details.tpl#L109
  https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/templates/frontend/objects/article_details.tpl#L191-L194
  https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/templates/frontend/objects/article_fulltext.tpl#L94-L97.
• It seems the DOI URL is wrongly escaped when assigning to the smarty variable here https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/templates/frontend/objects/article_details.tpl#L83, instead on display. I see in the default theme and in the main code it is the same. I will collect all the cases in the default theme in a separate issue to address them.
• Also here the escape is not used on output but withing if statement: https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/templates/frontend/components/footer.tpl#L39.
• <dl> instead of </dl> here https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/templates/frontend/objects/article_details.tpl#L89.
• Google Fonts loaded via @import url('https://fonts.googleapis.com/...') in styles/smw-font.less — GDPR concern (reader IPs sent to Google). Could we self-host the font files?

Also:

• Duplicate @import 'header.less'; in https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/styles/index.less.
• Large inline <style>/<script> blocks could maybe live in separate files?
• Inconsistent GPL v2/v3 license headers across files

I am not sure about those JS issues, so maybe also here to ask Jarda.

Thanks a lot!

[kaitlinnewson]

Similar to the Ammonite plugin, this requires the user to have installed and enabled the Immersion theme to function properly, so that needs to be documented in the description and perhaps handled in the code (e.g. don't let the user set this theme without that perhaps?).

[Godoy0722]

Hello @bozana and @kaitlinnewson! I just addressed your suggestions into the theme and generated a new release with these fixes! Would you mind having a second look? I appreciate it! Thanks a lot.

[bozana]

Hi @Godoy0722,

I have a few comments:

• https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/templates/frontend/components/footer.tpl#L26: pageFooter needs escaping.
• https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/js/htmlGalley.js#L77: querySelectorAll returns a NodeList, which has no .remove() method.
• maybe to consider data: here https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/js/htmlGalley.js#L51-L54: In order not to strip legitimate inline images in src, maybe to only block data: on href (links), and only block data:text/html and data:image/svg on src:

  if (name === 'href' && /^\s*(javascript|data):/i.test(value)) {                                                                                                                                                                                             
      el.removeAttribute(name);                                                                                                                                                                                                                               
      continue;
  }                                                                                                                                                                                                                                                           
  if ((name === 'src' || name === 'xlink:href') &&
      /^\s*(javascript:|data:(text\/html|image\/svg))/i.test(value)) {                                                                                                                                                                                        
      el.removeAttribute(name);
  }               

This keeps innocent data:image/png inline images working while blocking the dangerous cases

• some German comments here: https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/templates/frontend/objects/article_fulltext.tpl#L106 and https://github.com/Godoy0722/smwImmersion/blob/stable-3_5_0/templates/plugins/generic/htmlArticleGalley/templates/display.tpl#L32.

It looks great! You have considered everything!

This plugin.xml would also need the rebase then.
Thanks a lot!!!

[Godoy0722]

Hello again @bozana! First of all, sorry for the very late on answering you here! Just a heads-up that I updated the theme with your requested changes! Please feel free to have a look at it when you can! Thank you.

[bozana]

3.5.0.0 or 3.5.0.3 or 3.5.0.4?

[bozana]

Hi @Godoy0722, thanks a lot! Just one more small thing -- the version the plugin is compatible with should be fixed in plugins.xml, I think -- see my last comment here.
Thanks a lot!

[Godoy0722]

Hello @bozana ! Please let me know if I did it correctly with my most recent change! :)