search

Asgaros / asgaros-forum

Asgaros Forum WordPress plugin repository.
https://asgaros.com
GNU General Public License v2.0
90 stars 34 forks source link

tag-like thread titles are corrupted/stripped, like test<a> becomes test #355

Open divinity76 opened 2 years ago

divinity76 commented 2 years ago

if you try to make a thread with the title test<a> , the <a> part is stripped, causing data corruption in the title, it erroneously becomes test

divinity76 commented 2 years ago

fwiw this kind of bug most often happens because people try to "sanitize" strings before inserting them in a database. rule-of-thumbs:

DO: sql-encode strings before inserting them in a database (or better yet, use prepared statements - prevents SQL injection) DO NOT: strip strings before inserting them (that leads to data corruption like above) DO: html-encode strings before spiting them back out (prevents XSS/javascript-injection) DO NOT: strip strings before spitting them back out (leads to data corruption like above)

Asgaros commented 1 year ago

Hello @divinity76

I took a look and I think it is fine the way it is. Usually I agree that contents should not just get stripped away, but I think there is not really a useful scenario where it makes sense to place HTML-tags within the title.

For your refernce: The input gets sanitized by using the sanitize_text_field() function of WordPress which strips all tags: https://developer.wordpress.org/reference/functions/sanitize_text_field/

divinity76 commented 1 year ago

@Asgaros i disagree, it's not fine. it makes Asgaros forum unsuitable for programming forums. Take for example this thread from Stackoverflow: https://stackoverflow.com/questions/818255/what-does-21-mean/818284#818284

The title is What does " 2>&1 " mean? (because in Bash scripting, 2 >&1 means "redirect stderr to stdout")

imagine what that thread title would look like in a Asgaros forum, the title would become What does " 2 &1 " mean? (which in bash scripting has a COMPLETELY different meaning)

Asgaros commented 1 year ago

Good point. I will take a look into it again and will see how I can change this without causing another round of lengthy discussions during a security review from the WordPress plugin-repository team.