picandocodigo / List-Category-Posts

WordPress plugin which allows you to list posts from a category into a post/page using the [catlist] shortcode.
http://wordpress.org/extend/plugins/list-category-posts/
GNU General Public License v2.0
241 stars 112 forks source link

Potential cross-site scripting vulnerability #501

Closed VernonGrant closed 8 months ago

VernonGrant commented 1 year ago

Hi, we discovered a potential cross-site scripting vulnerability in regards to the pagination lcp_page0 query parameter. To be clear, the injected script is still hex encoded on the client side, so there's no immediate risk at this stage. Regardless, the result is an unnecessary risk.

Expected behavior

I would expect additional non numerical information that's passed to the lcp_page0 query parameter to be disregarded. So when I pass the following value as a page number:

https://example-domain/sub/?lcp_page0=3<%00script>alert(219);</script%00>

The pagination output should look like this:

<ul class='lcp_paginator'>
  <li><a href='https://example-domain/sub/?lcp_page0=2#lcp_instance_0' title='2' class='lcp_prevlink'></a></li>
  <li><a href='https://example-domain/sub/?lcp_page0=1#lcp_instance_0' title='1'>1</a></li>
  <li><a href='https://example-domain/sub/?lcp_page0=2#lcp_instance_0' title='2'>2</a></li>
  <li class='lcp_currentpage'>3</li>
  <li><a href='https://example-domain/sub/?lcp_page0=4#lcp_instance_0' title='4'>4</a></li>
  <li><a href='https://example-domain/sub/?lcp_page0=5#lcp_instance_0' title='5'>5</a></li>
  <li><a href='https://example-domain/sub/?lcp_page0=4#lcp_instance_0' title='4' class='lcp_nextlink'></a></li>
</ul>

Actual behavior

But the actual behavior for the following:

https://example-domain/sub/?lcp_page0=3<%00script>alert(219);</script%00>

Is having the additional data added to the pagination links.

<ul class='lcp_paginator'>
  <li><a href='https://example-domain/sub/?%3C%00script%3Ealert(219);%3C/script%00%3E&lcp_page0=2#lcp_instance_0' title='2' class='lcp_prevlink'></a></li>
  <li><a href='https://example-domain/sub/?%3C%00script%3Ealert(219);%3C/script%00%3E&lcp_page0=1#lcp_instance_0' title='1'>1</a></li>
  <li><a href='https://example-domain/sub/?%3C%00script%3Ealert(219);%3C/script%00%3E&lcp_page0=2#lcp_instance_0' title='2'>2</a></li>
  <li class='lcp_currentpage'>3</li>
  <li><a href='https://example-domain/sub/?%3C%00script%3Ealert(219);%3C/script%00%3E&lcp_page0=4#lcp_instance_0' title='4'>4</a></li>
  <li><a href='https://example-domain/sub/?%3C%00script%3Ealert(219);%3C/script%00%3E&lcp_page0=5#lcp_instance_0' title='5'>5</a></li>
  <li><a href='https://example-domain/sub/?%3C%00script%3Ealert(219);%3C/script%00%3E&lcp_page0=4#lcp_instance_0' title='4' class='lcp_nextlink'></a></li>
</ul>

Versions

klemens-st commented 9 months ago

With the changes implemented in v0.89.4 in the pagination code: https://github.com/picandocodigo/List-Category-Posts/commit/2af44e364a83fafbd17f385c06267e5f57750bec#diff-c6c0a5f1ff13fafc11629987ac884d68a0784f66a2e0251821fac4f10e11bc09L137-R137

the generated url is safe no matter what characters are added to it, this means the issue described above is not a security vulnerability. Nevertheless, to make it work cleaner, we will still implement validation proposed in #502