voidlabs / mosaico

Mosaico - Responsive Email Template Editor
https://mosaico.io
GNU General Public License v3.0
1.71k stars 503 forks source link

Replace toastr with sweetalert2 for toast notifications #658

Closed chris-tuncap closed 2 years ago

chris-tuncap commented 2 years ago

Toastr is not actively being maintained and Snyk has reported a vulnerability that has no fix: Issues with no direct upgrade or patch: ✗ Cross-site Scripting (XSS) [High Severity][https://snyk.io/vuln/SNYK-JS-TOASTR-2396430] in toastr@2.1.4 introduced by toastr@2.1.4 No upgrade or patch available

bago commented 2 years ago

1) We already use toastr in a non vulnerable way ( https://github.com/voidlabs/mosaico/commit/36f8ad8ed5b97434a353cfd9f6204b5ec07a933f ), and, we didn't pass user-submitted content to the notifications, so I guess the XSS was not applicable even before this patch, unless you load master templates or language packs from untrusted sources (but in that case I'd be warried XSS issues already exists in main mosaico) 2) You can replace toastr by using a viewModel plugin, with no changes at all to the mosaico codebase or any other existing extension that uses the viewModel.notifier error|success|info methods.

Just create a new viewModel plugin ( https://github.com/voidlabs/mosaico/wiki/Mosaico-Plugins#viewmodel-plugins ) And replace viewModel.notifier object with your own calling sweetalert.

e.g:

viewModel.notifier = {
  'error': function(msg) { sweetAlert.fire({ icon: 'error', title: msg });  },
  'success': function(msg) { sweetAlert.fire({ icon: 'success', title: msg });  },
  'info': function(msg) { sweetAlert.fire({ icon: 'info', title: msg });  },  
};

PS: I'll think better about this but I think toastr is a better UX solution for mosaico (e.g: I gave a very fast read to sweetalert2 docs and I guess one of the missing options from sweetalert is being able to stack multiple notifications.)