PHPBB3-16250 - Improve detection of unsafe BBCodes in ACP

Discuss requests for comments/changes posted in the Issue Tracker for the development of phpBB. Current releases are 3.2/Rhea and 3.3/Proteus.
Post Reply
User avatar
JoshyPHP
Registered User
Posts: 381
Joined: Fri Jul 08, 2011 9:43 pm

PHPBB3-16250 - Improve detection of unsafe BBCodes in ACP

Post by JoshyPHP »

https://tracker.phpbb.com/browse/PHPBB3-16250
https://github.com/phpbb/phpbb/pull/5770

Some context: when adding or editing a custom BBCode, the form produces an error if the BBCode is deemed risky. The user is warned but can still create the risky BBCode. The error message mentions the possibility of an XSS issue but I don't think that can ever be the case. It doesn't really matter though.

This PR removes the safeness check from the ACP module and adds a new service that can analyse a BBCode's definition to return an array with its findings. Excerpt from the interface:
* Required elements in the return array:
* - status:
* - "safe" The BBCode is valid and can be safely used by anyone.
* - "unsafe" The BBCode is valid but may be unsafe to use.
* - "invalid_definition" There is an issue with the definition.
* - "invalid_template" There is an issue with the template.
*
* Optional elements in the return array:
* - name: Name of the BBCode based on the definition. Required if status is "safe".
* - error_text: Textual description of the issue in plain text or as a L_* string.
* - error_html: Visual description of the issue in HTML.
As of the current PR, only the status and name values are used in the ACP. The other fields are populated when data is available. error_text contains an error message in plain English from the s9e\TextFormatter library. Maybe it could be displayed as supplementary info in addition to phpBB's default error message. The error_html field contains a HTML representation of the template with the problematic part highlighted. You can see an example of it in this doc. I didn't use it because I couldn't find a clean way to display it in the ACP error message.

Post Reply