CervantesSec/cervantes

Improper handling of Scriban conditional operators in Report Components

mseckar opened this issue · 12 comments

mseckar commented

When generating a Report using a Report Template containing a Report Component with Scriban conditional operator >, <, >=, <= or &&, the UI freezes and we obtain the following error:

System.InvalidOperationException: This template has errors. 
Check the <Template.HasError> and <Template.Messages> before evaluating a template. 
Messages: <input>(18,28) : error : Invalid token found `&`. 
Expecting <EOL>/end of line. at Scriban.Template.CheckErrors() at 
Scriban.Template.EvaluateAndRenderAsync(TemplateContext context, Boolean render) at 
Scriban.Template.RenderAsync(TemplateContext context) at 
Cervantes.Web.Controllers.ReportController.GenerateNewReport(ReportCreateViewModel model) in 
/src/Cervantes.Web/Controllers/ReportController.cs:line 851

We believe that the operator signs < and > are being sanitized to &lt; and &gt;, respectively. This can be seen by opening the Source Code view of the Report Component in TinyMCE.

Steps to reproduce:

  • Create a Report Component containing the following string:
{{if 1 < 2}}
test
{{else}}
test2
{{end}}
  • Attach the Component to a Report template
  • Create a Report using the aforementioned Report template
  • The UI will become unresponsive
  • Check the logs for error message

Creds to @jstangle for the initial discovery.

mesquidar commented

Hi @mseckar and @jstangle thanks for reporting the issue I will take it look and I will let you know :)

Thank you
Best regards

  • 👍2
mesquidar commented

Hi @mseckar and @jstangle I found the issue and I think is fixed I created a dev image if you want to test it before to commit the changes :)

git clone https://github.com/CervantesSec/docker.git
  • After that you need to start your docker containers:
docker-compose -p cervantes -f docker-compose-dev.yml up -d

Thank you :)
Best regards

  • 🎉1
jstangle commented

Hi @mesquidar, thank you for your quick solution. I can confirm it is working. I will do some additional tests and let you know.

Regards,
Jan

mseckar commented

Hello @mesquidar, the fix LGTM, all operators pass.

Thanks for the quick response.
Best regards

mesquidar commented

Perfect thank you both for confirming, I will commit the changes and close the issue :)

Best regards

  • 👍2
jpostolk commented

Hi @mesquidar, unfortunately this bug seems to be back again. Old unedited components and templates work fine but new ones with conditional operators behave the same as before.

mesquidar commented

Hi @jpostolk let me take it a look :)

mesquidar commented

Hi @jpostolk It should be fixed again. When we updated the sanitizer method it broke the components part again.
There's already a new image available with fix :)

Thank you
Best regards

  • 👍2
jpostolk commented

Hi, thank you very much for the quick fix :)

jpostolk commented

Hi @mesquidar, unfortunately this seems to be broken again.

Best regards,
Jonas

mesquidar commented

Hi @jpostolk I checked it and this time it seems it was Premailer.net (library that process and move the css inline) that encodes the characters and makes scriban break. It should be fixed again and a new image it's already available :)

Thank you
Best regards

  • ❤️2
jstangle commented

Thanks @mesquidar, we have verified the fix, and it is now working well. :)