Merge pull request #500 from HubSpot/feature/xss-escaping-modules

Feature/xss escaping modules
HubSpot · Feb 26, 2024 · eed3840 · eed3840
2 parents 237de6b + d9812e5
commit eed3840
Show file tree

Hide file tree

Showing 11 changed files with 55 additions and 46 deletions.
diff --git a/src/modules/button.module/module.html b/src/modules/button.module/module.html
@@ -60,10 +60,10 @@
 {# Button #}
 
 <div class="button-wrapper">
-  <a class="button" href="{{ href }}"
+  <a class="button" href="{{ href|escape_url }}"
   {% if module.link.open_in_new_tab %}target="_blank"{% endif %}
-  {% if rel %}rel="{{ rel|join(" ") }}"{% endif %}
+  {% if rel %}rel="{{ rel|join(" ")|escape_attr }}"{% endif %}
   >
-    {{ module.button_text }}
+    {{ module.button_text|escape_html }}
   </a>
 </div>
diff --git a/src/modules/card.module/module.html b/src/modules/card.module/module.html
@@ -42,18 +42,18 @@
   {% for card in module.card %}
     <section class="cards__card card">
       {% if card.image.src %}
-        {% set sizeAttrs = 'width="{{ card.image.width }}" height="{{ card.image.height }}"' %}
+        {% set sizeAttrs = 'width="{{ card.image.width|escape_attr }}" height="{{ card.image.height|escape_attr }}"' %}
         {% if card.image.size_type == "auto" %}
           {% set sizeAttrs = 'style="max-width: 100%; height: auto;"' %}
         {% elif card.image.size_type == "auto_custom_max" %}
-          {% set sizeAttrs = 'width="{{ card.image.max_width }}" height="{{ card.image.max_height }}" style="max-width: 100%; max-height: auto;"' %}
+          {% set sizeAttrs = 'width="{{ card.image.max_width|escape_attr }}" height="{{ card.image.max_height|escape_attr }}" style="max-width: 100%; max-height: auto;"' %}
         {% endif %}
         {% set loadingAttr = card.image.loading != "disabled" ? 'loading="{{ card.image.loading }}"' : "" %}
-        <img class="card__image" src="{{ card.image.src }}" alt="{{ card.image.alt }}" {{ loadingAttr }} {{ sizeAttrs }}>
+        <img class="card__image" src="{{ card.image.src|escape_url }}" alt="{{ card.image.alt|escape_attr }}" {{ loadingAttr }} {{ sizeAttrs }}>{# loadingAttr, sizeAttrs escaped above #}
       {% endif %}
       {% if card.text %}
         <div class="card__text">
-          {{ card.text }}
+          {{ card.text|sanitize_html }}
         </div>
       {% endif %}
     </section>

diff --git a/src/modules/menu.module/module.html b/src/modules/menu.module/module.html
@@ -86,24 +86,33 @@
   {% set menu = menu(module.menu, "site_root").children %}
 
   {% macro render_link_item(link, depth) %}
-    <li class="menu__item menu__item--depth-{{ depth }} {{ "menu__item--has-submenu" if link.children and depth < module.max_levels }} hs-skip-lang-url-rewrite">
+    <li class="menu__item menu__item--depth-{{ depth|escape_attr }} {{ "menu__item--has-submenu" if link.children and depth < module.max_levels }} {# outputting string no need to escape #}  hs-skip-lang-url-rewrite">
       {% if link.url %}
-        <a class="menu__link {{ "menu__link--toggle" if link.children and depth < module.max_levels }} {{ "menu__link--active-branch" if link.activeBranch }} {{ "menu__link--active-link" if link.activeNode }}" href="{{ link.url }}" {{ 'aria-haspopup="true" aria-expanded="false"' if link.children and depth < module.max_levels }} {{ 'aria-current="page"' if link.activeNode }} {{ 'target="_blank" rel="noopener"' if link.linkTarget }}>{{ link.label }}</a>
+        <a class="menu__link
+          {{ "menu__link--toggle" if link.children and depth < module.max_levels }}{# outputting string no need to escape #}
+          {{ "menu__link--active-branch" if link.activeBranch }}{# outputting string no need to escape #}
+          {{ "menu__link--active-link" if link.activeNode }}" {# outputting string no need to escape #}
+          href="{{ link.url|escape_url }}"
+          {{ 'aria-haspopup="true" aria-expanded="false"' if link.children and depth < module.max_levels }} {# outputting string no need to escape #}
+          {{ 'aria-current="page"' if link.activeNode }} {# outputting string no need to escape #}
+          {{ 'target="_blank" rel="noopener"' if link.linkTarget }}> {# outputting string no need to escape #}
+            {{ link.label|escape_html }}
+        </a>
       {% else %}
         {% if link.children and depth < module.max_levels %}
-          <a class="menu__link menu__link--toggle" href="#" aria-haspopup="true" aria-expanded="false">{{ link.label }}</a>
+          <a class="menu__link menu__link--toggle" href="#" aria-haspopup="true" aria-expanded="false">{{ link.label|escape_html }}</a>
         {% else %}
-          <span class="menu__link">{{ link.label }}</span>
+          <span class="menu__link">{{ link.label|escape_html }}</span>
         {% endif %}
       {% endif %}
       {% if link.children %}
         {% if depth and depth < module.max_levels %}
+          {% set depth = depth + 1 %}
           <button class="menu__child-toggle no-button" aria-expanded="false">
-            <span class="show-for-sr">Show submenu for {{ link.label }}</span>
+            <span class="show-for-sr">Show submenu for {{ link.label|escape_html }}</span>
             <span class="menu__child-toggle-icon"></span>
           </button>
-          <ul class="menu__submenu menu__submenu--level-{{ depth + 1 }} no-list">
-            {% set depth = depth + 1 %}
+          <ul class="menu__submenu menu__submenu--level-{{ depth|escape_attr }} no-list">
             {% for sublink in link.children %}
               {{ render_link_item(sublink, depth) }}
             {% endfor %}

diff --git a/src/modules/pricing-card.module/module.html b/src/modules/pricing-card.module/module.html
@@ -127,10 +127,10 @@
   {% if module.tier and module.description %}
     <div class="card__header">
       {% if module.tier %}
-        {{ module.tier }}
+        {{ module.tier|sanitize_html }}
       {% endif %}
       {% if module.description %}
-        {{ module.description }}
+        {{ module.description|sanitize_html }}
       {% endif %}
     </div>
   {% endif %}
@@ -145,22 +145,22 @@
             purpose="decorative",
             style="{{ module.feature_icon.type }}",
             unicode="{{ module.feature_icon.unicode }}"
-          %}{{ feature }}
+          %}{{ feature|escape_html }}
         </li>
       {% endfor %}
     </ul>
     <hr class="card__hr">
-    <p class="card__price">{{ module.price }}{{ module.timeframe }}</p>
+    <p class="card__price">{{ module.price|escape_html }}{{ module.timeframe|escape_html }}</p>
     <div class="card__button-wrapper button-wrapper">
       {% if has_overlay %}
-        <button class="card__button button button__open-overlay-{{ name }}">
+        <button class="card__button button button__open-overlay-{{ name|escape_attr }}">
       {% else %}
-        <a href="{{ href }}"
+        <a href="{{ href|escape_url }}"
           class="card__button button"
           {{ module.button_link.open_in_new_tab or has_payment_link ? 'target="_blank"' : "" }}
-          {{ rel ? 'rel="{{ rel|join(" ") }}"' : "" }}>
+          {{ rel ? 'rel="{{ rel|join(" ")|escape_attr }}"' : "" }}>
       {% endif %}
-          {{ module.button_text }}
+          {{ module.button_text|escape_html }}
       {% if has_overlay %}
         </button>
       {% else %}
@@ -171,25 +171,25 @@
 </section>
 
 {% if has_overlay %}
-  <div class="card__overlay-background" data-instance="{{ name }}" style="display: none;"></div>
-  <div class="card__overlay" data-instance="{{ name }}" data-disabled="{{ is_in_editor }}" style="display: none;">
+  <div class="card__overlay-background" data-instance="{{ name|escape_attr }}" style="display: none;"></div>
+  <div class="card__overlay" data-instance="{{ name|escape_attr }}" data-disabled="{{ is_in_editor|escape_attr }}" style="display: none;">
     <button class="card__overlay-close--top">
       <svg version="1.1" xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 32 28" aria-hidden="true" focusable="false">
         <path d="M2.71 26.79c0.312 0.314 0.743 0.508 1.22 0.508s0.908-0.194 1.22-0.507l10.85-10.85 10.86 10.85c0.312 0.312 0.744 0.505 1.22 0.505 0.953 0 1.725-0.772 1.725-1.725 0-0.476-0.193-0.908-0.505-1.22v0l-10.86-10.85 10.85-10.86c0.279-0.305 0.45-0.713 0.45-1.16 0-0.95-0.77-1.72-1.72-1.72-0.448 0-0.856 0.171-1.162 0.452l0.001-0.001-10.86 10.85-10.86-10.85c-0.316-0.344-0.767-0.56-1.27-0.56-0.95 0-1.72 0.77-1.72 1.72 0 0.502 0.215 0.954 0.558 1.268l0.001 0.001 10.85 10.86-10.85 10.86c-0.311 0.311-0.503 0.741-0.503 1.215s0.192 0.904 0.503 1.215l-0-0z"></path>
       </svg>
-      <span class="show-for-sr">{{ module.default_text.close_checkout }}</span>
+      <span class="show-for-sr">{{ module.default_text.close_checkout|escape_html }}</span>
     </button>
 
     <!-- Start of payments embed script -->
 
-    <div class="payments-iframe-container" data-src="{{ data_src }}"></div>
+    <div class="payments-iframe-container" data-src="{{ data_src|escape_url }}"></div>
     <script type="text/javascript" src="https://static.hsappstatic.net/payments-embed/ex/PaymentsEmbedCode.js"></script>
 
     <!-- End of payments embed script -->
 
     {# A focusable element is required to come after the iframe in order to trap focus in the overlay #}
 
-    <button class="card__overlay-close--bottom">{{ module.default_text.close_checkout }}</button>
+    <button class="card__overlay-close--bottom">{{ module.default_text.close_checkout|escape_html }}</button>
   </div>
 {% endif %}
 

diff --git a/src/modules/social-follow.module/module.html b/src/modules/social-follow.module/module.html
@@ -114,9 +114,9 @@
 
     {# Icon #}
 
-    <a class="social-links__link" href="{{ href }}"
+    <a class="social-links__link" href="{{ href|escape_url }}"
     {% if item.social_link.open_in_new_tab %}target="_blank"{% endif %}
-    {% if rel %}rel="{{ rel|join(" ") }}"{% endif %}>
+    {% if rel %}rel="{{ rel|join(" ")|escape_attr }}"{% endif %}>
       {% icon
         extra_classes="social-links__icon",
         name="{{ social_icon }}",

diff --git a/src/templates/blog-post.html b/src/templates/blog-post.html
@@ -14,17 +14,17 @@
 
   <div class="content-wrapper">
     <article class="blog-post">
-      <h1>{{ content.name }}</h1>
+      <h1>{{ content.name|sanitize_html }}</h1>
       <div class="blog-post__meta">
-        <a href="{{ blog_author_url(group.id, content.blog_post_author.slug) }}" rel="author">
-          {{ content.blog_post_author.display_name }}
+        <a href="{{ blog_author_url(group.id, content.blog_post_author.slug)|escape_url }}" rel="author">
+          {{ content.blog_post_author.display_name|escape_html }}
         </a>
-        <time datetime="{{ content.publish_date }}" class="blog-post__timestamp">
-          {{ content.publish_date_localized }}
+        <time datetime="{{ content.publish_date|escape_attr }}" class="blog-post__timestamp">
+          {{ content.publish_date_localized|escape_html }}
         </time>
       </div>
       <div class="blog-post__body">
-        {{ content.post_body }}
+        {{ content.post_body }}{# escaped elsewhere #}
       </div>
       {% if content.tag_list %}
         <div class="blog-post__tags">
@@ -34,7 +34,7 @@ <h1>{{ content.name }}</h1>
             style="SOLID"
           %}
           {% for tag in content.tag_list %}
-            <a class="blog-post__tag-link" href="{{ blog_tag_url(group.id, tag.slug) }}" rel="tag">{{ tag.name }}</a>{% if not loop.last %},{% endif %}
+            <a class="blog-post__tag-link" href="{{ blog_tag_url(group.id, tag.slug) }}" rel="tag">{{ tag.name|escape_html }}</a>{% if not loop.last %},{% endif %}
           {% endfor %}
         </div>
       {% endif %}
@@ -60,16 +60,16 @@ <h1>{{ content.name }}</h1>
           <h2>Read On</h2>
           <div class="blog-related-posts__list">
     {% endif %}
-            <article class="blog-related-posts__post" aria-label="Blog post summary: {{ post.name }}">
+            <article class="blog-related-posts__post" aria-label="Blog post summary: {{ post.name|escape_attr }}">
               {% if post.featured_image %}
-                <a class="blog-related-posts__post-image-wrapper" href="{{ post.absolute_url }}" aria-label="{% if post.featured_image_alt_text %} Featured image: {{ post.featured_image_alt_text }} - {% endif %}Read full post: {{ post.name }}">
-                  <img class="blog-related-posts__image" src="{{ post.featured_image }}" loading="lazy" width="352" alt="{{ post.featured_image_alt_text }}">
+                <a class="blog-related-posts__post-image-wrapper" href="{{ post.absolute_url|escape_url }}" aria-label="{% if post.featured_image_alt_text %} Featured image: {{ post.featured_image_alt_text|escape_attr }} - {% endif %}Read full post: {{ post.name|escape_attr }}">
+                  <img class="blog-related-posts__image" src="{{ post.featured_image|escape_url }}" loading="lazy" width="352" alt="{{ post.featured_image_alt_text|escape_attr }}">
                 </a>
               {% endif %}
               <div class="blog-related-posts__content">
                 <h3 class="blog-related-posts__title">
-                  <a class="blog-related-posts__title-link" href="{{ post.absolute_url }}">{{ post.name }}</a></h3>
-                {{ post.post_summary|truncatehtml(100) }}
+                  <a class="blog-related-posts__title-link" href="{{ post.absolute_url|escape_url }}">{{ post.name|escape_html }}</a></h3>
+                {{ post.post_summary|truncatehtml(100)|sanitize_html("STRIP") }}
               </div>
             </article>
       {% if count == total %}

diff --git a/src/templates/system/membership-login.html b/src/templates/system/membership-login.html
@@ -34,7 +34,7 @@
         path="@hubspot/rich_text"
       %}
         {% module_attribute "html" %}
-          <p>Having trouble? <a href="{{"mailto:" ~ site_settings.membershipWebsiteAdmin}}">Contact the admin</a>.</p>
+          <p>Having trouble? <a href="{{ ("mailto:" ~ site_settings.membershipWebsiteAdmin)|escape_url }}">Contact the admin</a>.</p>
         {% end_module_attribute %}
       {% end_module_block %}
     </div>

diff --git a/src/templates/system/membership-register.html b/src/templates/system/membership-register.html
@@ -33,7 +33,7 @@
         path="@hubspot/rich_text"
       %}
         {% module_attribute "html" %}
-          <p>Having trouble? <a href="{{"mailto:" ~ site_settings.membershipWebsiteAdmin}}">Contact the admin</a>.</p>
+          <p>Having trouble? <a href="{{ ("mailto:" ~ site_settings.membershipWebsiteAdmin)|escape_url }}">Contact the admin</a>.</p>
         {% end_module_attribute %}
       {% end_module_block %}
     </div>

diff --git a/src/templates/system/membership-reset-password-request.html b/src/templates/system/membership-reset-password-request.html
@@ -31,7 +31,7 @@
         path="@hubspot/rich_text"
       %}
         {% module_attribute "html" %}
-          <p>Having trouble? <a href="{{"mailto:" ~ site_settings.membershipWebsiteAdmin}}">Contact the admin</a>.</p>
+          <p>Having trouble? <a href="{{ ("mailto:" ~ site_settings.membershipWebsiteAdmin)|escape_url }}">Contact the admin</a>.</p>
         {% end_module_attribute %}
       {% end_module_block %}
     </div>

diff --git a/src/templates/system/membership-reset-password.html b/src/templates/system/membership-reset-password.html
@@ -32,7 +32,7 @@
         path="@hubspot/rich_text"
       %}
         {% module_attribute "html" %}
-          <p>Having trouble? <a href="{{"mailto:" ~ site_settings.membershipWebsiteAdmin}}">Contact the admin</a>.</p>
+          <p>Having trouble? <a href="{{ ("mailto:" ~ site_settings.membershipWebsiteAdmin)|escape_url }}">Contact the admin</a>.</p>
         {% end_module_attribute %}
       {% end_module_block %}
     </div>

diff --git a/src/templates/system/search-results.html b/src/templates/system/search-results.html
@@ -25,7 +25,7 @@
         path='@hubspot/rich_text'
       %}
         {% module_attribute 'html' %}
-          <h1>Results for &ldquo;{{ search_query|escape }}&rdquo;</h1>
+          <h1>Results for &ldquo;{{ search_query|escape_html }}&rdquo;</h1>
         {% end_module_attribute %}
       {% end_module_block %}