crystal icon indicating copy to clipboard operation
crystal copied to clipboard

Should ECR provide support for escaping rendered output?

Open robacarp opened this issue 1 year ago • 3 comments

The docs for ECR state:

Embedded Crystal (ECR) is a template language for embedding Crystal code into other text, that includes but is not limited to HTML.

A naïve user (me) might assume that ECR is smart enough to escape syntax so that it doesn't end up causing an XSS vector on a HTML based project. My workaround for this has been laborious, and it includes manually sanitizing anything being rendered with a call to HTML.escape. The workflow is error prone because it requires me to remember to be a good steward of user input. In todays world, web frameworks have evolved to do this for me because it's been shown over and over again that I will fail to remember to do this 100% of the time.

This has led me to consider a bunch of web specific templating languages. Unfortunately there's a lot of bit-rot and abandoned projects in the shardverse, and I'm skiddish to attach my project to another which would require a rewrite when the author no longer has time for a passion project. So I keep coming back to ECR because it's built-in to crystal.

Would it be possible to upgrade ECR so that it can take a sanitation helper?

I'm humbly attempting to survey the ECR Processor for an easy victory. It seems like maybe wrapping this buffer_name in a call to a theoretical user_escape() method would suffice -- or maybe it would be elsewhere, I can't tell.

I'm inspired by the syntax for Log -- and I love it. Log = ::Log.for(self) is clean and elegant.

I'm imagining something like this, which would require a broader rework of ECR, but could be really powerful:

HtmlSafeECR = ECR.with_escape do |string|
  HTML.escape string
end

HtmlSafeECR.render template.ecr

ECR.with_escape would capture the block and apply it every time a <%= %> sequence is evaluated, providing a safe default render macro.

Instructing the escape block that the string is already escaped and shouldn't be handled gets tricky. Perl had taint and I remember using that to accomplish this on some protoframework. Ruby has both taint and trust -- though Rails implements something completely differenthttps://api.rubyonrails.org/classes/ActiveSupport/SafeBuffer.html) for it's usage of ERB.

What do you think?

robacarp avatar Oct 11 '24 16:10 robacarp

~~Duplicate of https://github.com/crystal-lang/crystal/issues/13753?~~

NVM

Blacksmoke16 avatar Oct 11 '24 16:10 Blacksmoke16

Another approach which might be more helpful is to make ECR a generic, and the T is the buffer. So the current implementation is:

module ECR
  macro render(filename)
    ::String.build do |%io|
      ::ECR.embed({{filename}}, %io)
    end
  end
end

But it could also work like this:

module ECR(BufferClass)
  macro render(filename)
    BufferClass.build do |%io|
      ::ECR.embed({{filename}}, %io)
    end
  end
end

BufferClass must then have a build method, which yields an object (a buffer ingest object). The buffer ingest object must respond to <<.

That would allow more in depth inspection of safe/unsafe strings through some out-of-band syntax. The buffer ingest object could be made responsible for deciding what needs to be escaped and what doesn't.

robacarp avatar Oct 11 '24 17:10 robacarp

Also somewhat related is that ERB (or Rail's extension of ERB) has a <%== operator which doesn't run the html sanitation step. It's subtle, maybe even too subtle. Enough that there are suggestions to avoid it.

robacarp avatar Oct 11 '24 17:10 robacarp

Even if it's a breaking change, I'd want to make <%= escape html by default and add <%== for not escaping, with maybe adding an ameba rule to discourage using the latter.

nobodywasishere avatar Feb 21 '25 18:02 nobodywasishere