CVE-2024-21517: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: This affects versions of the package opencart/opencart from 4.0.0.0. A reflected XSS issue was identified in the redirect parameter of customer account/login route. An attacker can inject arbitrary HTML and Javascript into the page response. As this vulnerability is present in the account functionality it could be used to target and attack customers of the OpenCart shop.**Notes:**1) The fix for this vulnerability is incomplete
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2024-06-22T05:15Z
Repository: https://github.com/opencart/opencart
Language: Other
Files changed: upload/catalog/controller/account/login.php, upload/catalog/controller/cms/comment.php, upload/catalog/controller/startup/seo_url.php, upload/catalog/view/template/account/login.twig, upload/catalog/view/template/cms/comment.twig, upload/catalog/view/template/cms/comment_list.twig, upload/catalog/view/template/cms/comment_reply.twig
Commit date: 2024-01-05 11:26:12 +0800
Commit message: added login to comment

Diff: diff --git a/upload/catalog/controller/account/login.php b/upload/catalog/controller/account/login.php
index d292489368..6b7f0dadda 100644
--- a/upload/catalog/controller/account/login.php
+++ b/upload/catalog/controller/account/login.php
@@ -75,11 +75,11 @@ class Login extends \Opencart\System\Engine\Controller {
 		if (isset($this->session->data['redirect'])) {
 			$data['redirect'] = $this->session->data['redirect'];
 
 			unset($this->session->data['redirect']);
 		} elseif (isset($this->request->get['redirect'])) {
-			$data['redirect'] = urldecode($this->request->get['redirect']);
+			$data['redirect'] = $this->request->get['redirect'];
 		} else {
 			$data['redirect'] = '';
 		}
 
 		$this->session->data['login_token'] = oc_token(26);
@@ -184,13 +184,19 @@ class Login extends \Opencart\System\Engine\Controller {
 			// Create customer token
 			$this->session->data['customer_token'] = oc_token(26);
 
 			$this->model_account_customer->deleteLoginAttempts($this->request->post['email']);
 
+			if (isset($this->request->post['redirect'])) {
+				$redirect = urldecode($this->request->post['redirect']);
+			} else {
+				$redirect = '';
+			}
+
 			// Added strpos check to pass McAfee PCI compliance test (http://forum.opencart.com/viewtopic.php?f=10&t=12043&p=151494#p151295)
-			if (isset($this->request->post['redirect']) && str_starts_with(html_entity_decode($this->request->post['redirect'], ENT_QUOTES, 'UTF-8'), $this->config->get('config_url'))) {
-				$json['redirect'] = html_entity_decode($this->request->post['redirect'], ENT_QUOTES, 'UTF-8') . '&customer_token=' . $this->session->data['customer_token'];
+			if ($redirect && str_starts_with($redirect, $this->config->get('config_url'))) {
+				$json['redirect'] = $redirect . '&customer_token=' . $this->session->data['customer_token'];
 			} else {
 				$json['redirect'] = $this->url->link('account/account', 'language=' . $this->config->get('config_language') . '&customer_token=' . $this->session->data['customer_token'], true);
 			}
 		}
 
diff --git a/upload/catalog/controller/cms/comment.php b/upload/catalog/controller/cms/comment.php
index 334e2260f6..f0a8b1ba57 100644
--- a/upload/catalog/controller/cms/comment.php
+++ b/upload/catalog/controller/cms/comment.php
@@ -35,11 +35,11 @@ class Comment extends \Opencart\System\Engine\Controller {
 		} else {
 			$page = 1;
 		}
 
 		$data['logged'] = $this->customer->isLogged();
-		$data['login'] = $this->url->link('account/login', 'language=' . $this->config->get('config_language'));
+		$data['login'] = $this->url->link('account/login', 'language=' . $this->config->get('config_language') . '&page=' . $page . '&redirect=' . urlencode($this->url->link('cms/blog.info', 'language=' . $this->config->get('config_language') . '&article_id=' . $data['article_id'], true)));
 
 		$this->session->data['comment_token'] = oc_token(32);
 
 		// Create a login token to prevent brute force attacks
 		$data['comment_add'] = $this->url->link('cms/comment.add', 'language=' . $this->config->get('config_language') . '&article_id=' . $data['article_id'] . '&comment_token=' . $this->session->data['comment_token'], true);
@@ -178,11 +178,11 @@ class Comment extends \Opencart\System\Engine\Controller {
 		$data['results'] = sprintf($this->language->get('text_pagination'), ($comment_total) ? (($page - 1) * $limit) + 1 : 0, ((($page - 1) * $limit) > ($comment_total - $limit)) ? $comment_total : ((($page - 1) * $limit) + $limit), $comment_total, ceil($comment_total / $limit));
 
 		$data['refresh'] = $this->url->link('cms/comment.list', 'language=' . $this->config->get('config_language') . '&article_id=' . $article_id . '&page=' . $page, true);
 
 		$data['logged'] = $this->customer->isLogged();
-
+		$data['login'] = $this->url->link('account/login', 'language=' . $this->config->get('config_language') . '&redirect=' . urlencode($this->url->link('cms/blog.info', 'language=' . $this->config->get('config_language') . '&article_id=' . $article_id, true)));
 
 		return $this->load->view('cms/comment_list', $data);
 	}
 
 	/**
@@ -418,14 +418,24 @@ class Comment extends \Opencart\System\Engine\Controller {
 			$article_comment_id = (int)$this->request->get['article_comment_id'];
 		} else {
 			$article_comment_id = 0;
 		}
 
+		if (isset($this->request->get['rating'])) {
+			$rating = (bool)$this->request->get['rating'];
+		} else {
+			$rating = 0;
+		}
+
 		if (!isset($this->request->get['comment_token']) || !isset($this->session->data['comment_token']) || $this->request->get['comment_token'] != $this->session->data['comment_token']) {
 			$json['error'] = $this->language->get('error_token');
 		}
 
+		if (!$this->customer->isLogged()) {
+			$json['error'] = $this->language->get('error_login');
+		}
+
 		$this->load->model('cms/article');
 
 		$article_info = $this->model_cms_article->getArticle($article_id);
 
 		if (!$article_info) {
@@ -437,20 +447,16 @@ class Comment extends \Opencart\System\Engine\Controller {
 
 		if (!$article_comment_info) {
 			$json['error'] = $this->language->get('error_article_comment');
 		}
 
-		if (!$this->customer->isLogged() && !$this->config->get('config_comment_guest')) {
-			$json['error'] = $this->language->get('error_login');
-		}
-
 		if (!$json) {
 			// Anti-Spam
 			$rating_data = $this->request->post + [
 				'article_comment_id' => $article_comment_id,
 				'customer_id'        => $this->customer->getId(),
-				'rating'             => (bool)$this->request->get['rating'],
+				'rating'             => $rating,
 				'ip'                 => $this->request->server['REMOTE_ADDR']
 			];
 
 			$this->load->model('cms/article');
 
diff --git a/upload/catalog/controller/startup/seo_url.php b/upload/catalog/controller/startup/seo_url.php
index d67d67118e..2bfd57fef7 100644
--- a/upload/catalog/controller/startup/seo_url.php
+++ b/upload/catalog/controller/startup/seo_url.php
@@ -79,14 +79,14 @@ class SeoUrl extends \Opencart\System\Engine\Controller {
 
 		// Start changing the URL query into a path
 		$paths = [];
 
 		// Parse the query into its separate parts
-		$parts = explode('&', $url_info['query']);
+		$queries = explode('&', $url_info['query']);
 
-		foreach ($parts as $part) {
-			$pair = explode('=', $part);
+		foreach ($queries as $query) {
+			$pair = explode('=', $query);
 
 			if (isset($pair[0])) {
 				$key = (string)$pair[0];
 			}
 
diff --git a/upload/catalog/view/template/account/login.twig b/upload/catalog/view/template/account/login.twig
index e98b5280af..0402e9b75b 100644
--- a/upload/catalog/view/template/account/login.twig
+++ b/upload/catalog/view/template/account/login.twig
@@ -36,16 +36,16 @@
               <div class="mb-3">
                 <label for="input-password" class="col-form-label">{{ entry_password }}</label>
                 <input type="password" name="password" value="{{ password }}" placeholder="{{ entry_password }}" id="input-password" class="form-control mb-1"/>
                 <a href="{{ forgotten }}">{{ text_forgotten }}</a>
               </div>
-              <div class="text-end">
-                <button type="submit" class="btn btn-primary">{{ button_login }}</button>
-              </div>
               {% if redirect %}
                 <input type="hidden" name="redirect" value="{{ redirect }}"/>
               {% endif %}
+              <div class="text-end">
+                <button type="submit" class="btn btn-primary">{{ button_login }}</button>
+              </div>
             </form>
           </div>
         </div>
       </div>
       {{ content_bottom }}</div>
diff --git a/upload/catalog/view/template/cms/comment.twig b/upload/catalog/view/template/cms/comment.twig
index d5e8a66823..1d261f0351 100644
--- a/upload/catalog/view/template/cms/comment.twig
+++ b/upload/catalog/view/template/cms/comment.twig
@@ -1,12 +1,14 @@
 <div class="text-center">
-  <button type="button" value="{{ like }}" id="input-rating-like" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></button>
-  <button type="button" value="{{ dislike }}" id="input-rating-dislike" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></button>
   {% if logged %}
+    <button type="button" value="{{ like }}" id="input-rating-like" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></button>
+    <button type="button" value="{{ dislike }}" id="input-rating-dislike" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></button>
     <button type="button" value="{{ comment_add }}" data-oc-toggle="comment" data-oc-target="#comment-0" data-oc-trigger="#button-refresh" class="btn btn-secondary">{{ button_comment }}</button>
   {% else %}
-    <a href="" class="btn btn-secondary">{{ button_login }}</a>
+    <a href="{{ login }}" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></a>
+    <a href="{{ login }}" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></a>
+    <a href="{{ login }}" class="btn btn-secondary">{{ button_login }}</a>
   {% endif %}
 </div>
 <hr/>
 <div class="row mb-3">
   <div class="col">
diff --git a/upload/catalog/view/template/cms/comment_list.twig b/upload/catalog/view/template/cms/comment_list.twig
index 20dfe57919..e98e45f9f7 100644
--- a/upload/catalog/view/template/cms/comment_list.twig
+++ b/upload/catalog/view/template/cms/comment_list.twig
@@ -1,43 +1,35 @@
 <div id="comment-0">
   {% if comments %}
     {% for comment in comments %}
       <div id="comment-{{ comment.article_comment_id }}" class="border mb-3">
-
         <div class="p-2">
           <p>{{ text_by }} {{ comment.author }} - {{ comment.date_added }}</p>
           <p>{{ comment.comment }}</p>
           <div class="text-end">
-
-            <button type="button" value="{{ comment.like }}" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></button>
-            <button type="button" value="{{ comment.dislike }}" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></button>
-
-            {% if comment_guest %}
+            {% if logged %}
+              <button type="button" value="{{ comment.like }}" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></button>
+              <button type="button" value="{{ comment.dislike }}" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></button>
               <button type="button" value="{{ comment.reply_add }}" data-oc-toggle="comment" data-oc-target="#reply-{{ comment.article_comment_id }}" data-oc-trigger="#button-refresh-{{ comment.article_comment_id }}" class="btn btn-secondary">{{ button_reply }}</button>
             {% else %}
-              <button type="button" class="btn btn-secondary" disabled="disabled">{{ button_reply }}</button>
+              <a href="{{ login }}" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></a>
+              <a href="{{ login }}" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></a>
+              <a href="{{ login }}" class="btn btn-secondary">{{ button_login_reply }}</a>
             {% endif %}
-
           </div>
         </div>
-
         <div id="reply-{{ comment.article_comment_id }}">
-
           <div id="reply-{{ comment.article_comment_id }}-0"></div>
-
           <div class="text-center p-2 border-top">
-            <button type="button" value="{{ comment.reply }}" id="button-refresh-{{ comment.article_comment_id }}" data-oc-toggle="refresh" data-oc-target="#reply-{{ comment.article_comment_id }}-0" class="btn btn-secondary"><i class="fa fa-sync"></i></button>
-
+            <button type="button" value="{{ comment.reply }}" id="button-refresh-{{ comment.article_comment_id }}" data-oc-toggle="refresh" data-oc-target="#reply-{{ comment.article_comment_id }}-0" class="btn btn-secondary d-none"><i class="fa fa-sync"></i></button>
             {% if comment.reply_total %}
               <button type="button" value="{{ comment.reply }}" data-oc-toggle="next" class="btn btn-secondary">{{ button_replies }} ({{ comment.reply_total }})</button>
             {% else %}
               <button type="button" class="btn btn-secondary" disabled>{{ button_replies }}</button>
             {% endif %}
-
           </div>
         </div>
-
       </div>
     {% endfor %}
     <div class="row">
       <div class="col-sm-6 text-start">{{ pagination }}</div>
       <div class="col-sm-6 text-end">{{ results }}</div>
@@ -45,10 +37,10 @@
   {% else %}
     {{ text_no_results }}
   {% endif %}
 </div>
 <div class="text-center p-2">
-  <button type="button" value="{{ refresh }}" id="button-refresh" data-oc-toggle="refresh" data-oc-target="#comment-0" class="btn btn-secondary"><i class="fa fa-sync"></i></button>
+  <button type="button" value="{{ refresh }}" id="button-refresh" data-oc-toggle="refresh" data-oc-target="#comment-0" class="btn btn-secondary d-none"><i class="fa fa-sync"></i></button>
 </div>
 
 
 
diff --git a/upload/catalog/view/template/cms/comment_reply.twig b/upload/catalog/view/template/cms/comment_reply.twig
index 71566cd5c2..f0da2b85d2 100644
--- a/upload/catalog/view/template/cms/comment_reply.twig
+++ b/upload/catalog/view/template/cms/comment_reply.twig
@@ -7,11 +7,11 @@
       </div>
     {% endfor %}
   </div>
 {% endif %}
 <div class="text-center p-2 border-top">
-  <button type="button" value="{{ refresh }}" id="button-refresh-{{ parent_id }}" data-oc-toggle="refresh" data-oc-target="#reply-{{ parent_id }}-{{ page }}" class="btn btn-secondary"><i class="fa fa-sync"></i></button>
+  <button type="button" value="{{ refresh }}" id="button-refresh-{{ parent_id }}" data-oc-toggle="refresh" data-oc-target="#reply-{{ parent_id }}-{{ page }}" class="btn btn-secondary d-none"><i class="fa fa-sync"></i></button>
   {% if not replies %}
     <button type="button" class="btn btn-secondary" disabled>{{ button_replies }}</button>
   {% elseif next %}
     <button type="button" value="{{ next }}" data-oc-toggle="next" class="btn btn-secondary">{{ button_more }}</button>
   {% else %}

Vulnerable code: 			$data['redirect'] = urldecode($this->request->get['redirect']);
			if (isset($this->request->post['redirect']) && str_starts_with(html_entity_decode($this->request->post['redirect'], ENT_QUOTES, 'UTF-8'), $this->config->get('config_url'))) {
				$json['redirect'] = html_entity_decode($this->request->post['redirect'], ENT_QUOTES, 'UTF-8') . '&customer_token=' . $this->session->data['customer_token'];
		$data['login'] = $this->url->link('account/login', 'language=' . $this->config->get('config_language'));

		if (!$this->customer->isLogged() && !$this->config->get('config_comment_guest')) {
			$json['error'] = $this->language->get('error_login');
		}

				'rating'             => (bool)$this->request->get['rating'],
		$parts = explode('&', $url_info['query']);
		foreach ($parts as $part) {
			$pair = explode('=', $part);
              <div class="text-end">
                <button type="submit" class="btn btn-primary">{{ button_login }}</button>
              </div>
  <button type="button" value="{{ like }}" id="input-rating-like" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></button>
  <button type="button" value="{{ dislike }}" id="input-rating-dislike" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></button>
    <a href="" class="btn btn-secondary">{{ button_login }}</a>


            <button type="button" value="{{ comment.like }}" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></button>
            <button type="button" value="{{ comment.dislike }}" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></button>

            {% if comment_guest %}
              <button type="button" class="btn btn-secondary" disabled="disabled">{{ button_reply }}</button>




            <button type="button" value="{{ comment.reply }}" id="button-refresh-{{ comment.article_comment_id }}" data-oc-toggle="refresh" data-oc-target="#reply-{{ comment.article_comment_id }}-0" class="btn btn-secondary"><i class="fa fa-sync"></i></button>



  <button type="button" value="{{ refresh }}" id="button-refresh" data-oc-toggle="refresh" data-oc-target="#comment-0" class="btn btn-secondary"><i class="fa fa-sync"></i></button>
  <button type="button" value="{{ refresh }}" id="button-refresh-{{ parent_id }}" data-oc-toggle="refresh" data-oc-target="#reply-{{ parent_id }}-{{ page }}" class="btn btn-secondary"><i class="fa fa-sync"></i></button>
Fixed code: 			$data['redirect'] = $this->request->get['redirect'];
			if (isset($this->request->post['redirect'])) {
				$redirect = urldecode($this->request->post['redirect']);
			} else {
				$redirect = '';
			}

			if ($redirect && str_starts_with($redirect, $this->config->get('config_url'))) {
				$json['redirect'] = $redirect . '&customer_token=' . $this->session->data['customer_token'];
		$data['login'] = $this->url->link('account/login', 'language=' . $this->config->get('config_language') . '&page=' . $page . '&redirect=' . urlencode($this->url->link('cms/blog.info', 'language=' . $this->config->get('config_language') . '&article_id=' . $data['article_id'], true)));
		$data['login'] = $this->url->link('account/login', 'language=' . $this->config->get('config_language') . '&redirect=' . urlencode($this->url->link('cms/blog.info', 'language=' . $this->config->get('config_language') . '&article_id=' . $article_id, true)));
		if (isset($this->request->get['rating'])) {
			$rating = (bool)$this->request->get['rating'];
		} else {
			$rating = 0;
		}

		if (!$this->customer->isLogged()) {
			$json['error'] = $this->language->get('error_login');
		}

				'rating'             => $rating,
		$queries = explode('&', $url_info['query']);
		foreach ($queries as $query) {
			$pair = explode('=', $query);
              <div class="text-end">
                <button type="submit" class="btn btn-primary">{{ button_login }}</button>
              </div>
    <button type="button" value="{{ like }}" id="input-rating-like" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></button>
    <button type="button" value="{{ dislike }}" id="input-rating-dislike" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></button>
    <a href="{{ login }}" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></a>
    <a href="{{ login }}" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></a>
    <a href="{{ login }}" class="btn btn-secondary">{{ button_login }}</a>
            {% if logged %}
              <button type="button" value="{{ comment.like }}" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></button>
              <button type="button" value="{{ comment.dislike }}" data-oc-toggle="rate" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></button>
              <a href="{{ login }}" class="btn btn-secondary"><i class="fa fa-thumbs-up"></i></a>
              <a href="{{ login }}" class="btn btn-secondary"><i class="fa fa-thumbs-down"></i></a>
              <a href="{{ login }}" class="btn btn-secondary">{{ button_login_reply }}</a>
            <button type="button" value="{{ comment.reply }}" id="button-refresh-{{ comment.article_comment_id }}" data-oc-toggle="refresh" data-oc-target="#reply-{{ comment.article_comment_id }}-0" class="btn btn-secondary d-none"><i class="fa fa-sync"></i></button>
  <button type="button" value="{{ refresh }}" id="button-refresh" data-oc-toggle="refresh" data-oc-target="#comment-0" class="btn btn-secondary d-none"><i class="fa fa-sync"></i></button>
  <button type="button" value="{{ refresh }}" id="button-refresh-{{ parent_id }}" data-oc-toggle="refresh" data-oc-target="#reply-{{ parent_id }}-{{ page }}" class="btn btn-secondary d-none"><i class="fa fa-sync"></i></button>