Trading Buttons

:warning: security vulnerability :warning:

TL;DR installing this plugin will - even when disabled - leak all topic custom fields that are present to anyone who can access the topic, including anonymous users. Depending on other plugins you have installed, topic custom fields can contain sensitive data.

When vetting this plugin for a client we discovered a number of security issues. We have fixed these issues in our fork (https://github.com/communiteq/discourse-topic-trade-buttons/tree/master) and made a pull request. However, the topic author has not responded to our pull request or our PM so we are now disclosing these issues.

Security fix: information leakage

All custom fields (including those from other plugins!) are being serialized, including to anonymous users. Custom fields can contain sensitive data and should never be serialized like that.

Since the sold_at etc values are being set server side anyway and the buttons are “computed” on topic.archived, the custom field logic can be removed from the frontend user-facing code and the custom fields only need to be serialized for the admin interface to work - hence the serialization can be limited to admin users. We do suspect that this is not even necessary either.

Initialization fixes

The if SiteSetting.topic_trade_buttons_enabled check that is fencing the serialization logic makes it necessary to restart Discourse after enabling or disabling the plugin. This check is unnecessary since Discourse already takes care of that.
Using respect_plugin_enabled: false is unnecessary and aggravates the security issue described above.

7 Likes