Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom field and serialization fixes #14

Merged
merged 4 commits into from
Jun 23, 2024
Merged

Custom field and serialization fixes #14

merged 4 commits into from
Jun 23, 2024

Conversation

communiteq
Copy link
Contributor

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.

@jannolii jannolii merged commit a8fc41b into jannolii:master Jun 23, 2024
1 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants