Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested execution of {{ directive }} in html loaded via {{httpInclude}} #6745

Open
baiomys opened this issue Dec 11, 2024 · 14 comments
Open

Nested execution of {{ directive }} in html loaded via {{httpInclude}} #6745

baiomys opened this issue Dec 11, 2024 · 14 comments
Assignees
Labels
feature ⚙️ New feature or request

Comments

@baiomys
Copy link

baiomys commented Dec 11, 2024

Is it expected behavior?

How to disable running template code in HTML loaded via {{ httpInclude }} ?
It is not secure and could lead to unpredictable results.

@mholt
Copy link
Member

mholt commented Dec 11, 2024

Do you have an example of an httpInclude that is insecure?

The templates HTTP handler intentionally evaluates templates as interpretable HTML so that they can be used in HTML documents. (The intention is to template HTML pages, not just the text that appears within hard-coded HTML.)

I can think of ways that a template could be insecure by allowing untrusted input to be evaluated, but that is nothing new or special to Caddy templates.

@mholt mholt added the needs info 📭 Requires more information label Dec 11, 2024
@baiomys
Copy link
Author

baiomys commented Dec 11, 2024

If content loaded to [div id="email"] contain any {{ }} directives (intentionally or unintentionally), they will be executed as well, it should be avoided.

API call is substituted by UNTRUSTED static content:

Caddyfile

domain.tld {
  tls internal

  handle_path /static/* {
    rewrite * /{uri}
    reverse_proxy destination_static:8888 {
      @error status 520 500 404
      handle_response @error {
        respond "Nothing there" 200
      }
    }
  }

  handle_path /api/* {
    root * /etc/caddy/static/templates
    templates
    file_server
    rewrite * /template.html
  }
}

TEMPLATE

{{$mailid := (.OriginalReq.URL.Path | replace "/api/" "/static/") }}

<script>
   function ifload() {
     var iframe = document.getElementById("frame");
     iframe.contentWindow.document.body.innerHTML = document.getElementById("email").innerHTML;
   }
</script>

<div id="email">
   {{httpInclude $mailid}}
</div>

<iframe id="frame" src="" onload="ifload()"/>

@mohammed90

This comment has been minimized.

@baiomys
Copy link
Author

baiomys commented Dec 11, 2024

There's html function to which you can pipe the output of httpInclude. You can just do {{ httpInclude | html }}, and it'll escape the HTML out of the output.

I need to bypass execution of {{ directives }} in httpInclude content. Not escape html.
It seems Caddy engine process directives in loaded HTML code BEFORE pipelining, so this behavior can be switched off only by altering engine GO code.

To clarify situation, this is part of loaded via httpInclude content:

Logs

vzdump 9244 --mode snapshot --node reno --compress zstd --notes-template '{{guestname}}' --storage repo --remove 0 --notification-mode auto


9244: 2024-12-10 10:29:03 INFO: Starting Backup of VM 9244 (lxc)
9244: 2024-12-10 10:29:03 INFO: status = running

Caddy return error 520
function "guestname" not defined

@mohammed90
Copy link
Member

Ok, now I understand after seeing the example, and the issue title makes sense. Sorry for the confusion.

@mholt
Copy link
Member

mholt commented Dec 11, 2024

You can adjust the delimiters using the between subdirective: between <open_delim> <close_delim> That way you can change them from {{ }} to something else.

@baiomys
Copy link
Author

baiomys commented Dec 11, 2024

You can adjust the delimiters using the between subdirective: between <open_delim> <close_delim> That way you can change them from {{ }} to something else.

Loadable content is THOUSANDS of externally provided e-mail messages. So there are NO delimiters that can be treated as safe.
=)

Thanks for suggestion, I'll apply it as temporary solution, but creating version of httpIncludeNoscript would be much safer.

@mholt
Copy link
Member

mholt commented Dec 13, 2024

What's a good word other than "include" or "import" for that?

@mohammed90
Copy link
Member

What's a good word other than "include" or "import" for that?

How about includeRaw?

@mholt
Copy link
Member

mholt commented Dec 13, 2024

Yeah, that could work. I'll add this enhancement to my list. 👍

@mholt mholt self-assigned this Dec 13, 2024
@baiomys
Copy link
Author

baiomys commented Dec 13, 2024

Looks promising, tnx!

@baiomys baiomys closed this as completed Dec 13, 2024
@mohammed90 mohammed90 reopened this Dec 13, 2024
@mohammed90
Copy link
Member

Perhaps rawInclude is better to be analogous with httpInclude

@mohammed90 mohammed90 added feature ⚙️ New feature or request and removed needs info 📭 Requires more information labels Dec 13, 2024
@mholt
Copy link
Member

mholt commented Dec 14, 2024

Well, we might want a include variant as well, so I am leaning toward httpIncludeRaw and includeRaw if you're okay with that.

@baiomys
Copy link
Author

baiomys commented Jan 1, 2025

Hi, folks, Happy New Year!
Any progress on httpIncludeRaw ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants