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

Change the my_post_key regularly #3376

Closed
effone opened this issue Aug 3, 2018 · 2 comments · Fixed by #4022
Closed

Change the my_post_key regularly #3376

effone opened this issue Aug 3, 2018 · 2 comments · Fixed by #4022
Labels
b:1.8 Branch: 1.8.x s:resolved Status: Resolved. Solution implemented or scheduled t:enhancement Type: Enhancement. Contains minor improvements
Milestone

Comments

@effone
Copy link
Member

effone commented Aug 3, 2018

Quote from pre-PR:

MyBB uses "my_post_key" to prevent forged requests. Ideally this token should be unique for each request, or unique for each session (see on wikipedia) but in MyBB, the token never changes, ever.
At the same time it's easy for the my_post_key to be leaked. It's not only used in page sourcecode, as a hidden form input field, but also openly in links (Mark Forums Read, Star Rating, etc.) so a simple Copy & Paste operation may inadvertently include it, it's in the browser history, etc.
So here is an attempt to change the my_post_key regularly.
It adds a time-based element to the hash, causing a new code to be generated every 3 hours. By generating older codes in the verification step (It's an older code, sir, but it checks out...) each code may be valid for up to 12 hours. After that, the code is useless.
In terms of usability, this means if you send a post request with a tab that had been open for half a day, it will no longer work since the my_post_key of the old tab had already expired. You have to reload to get a new key.

PR for this:
#3374

@effone effone added t:enhancement Type: Enhancement. Contains minor improvements b:1.8 Branch: 1.8.x s:resolved Status: Resolved. Solution implemented or scheduled labels Aug 3, 2018
@effone effone added this to the 1.8.18 milestone Aug 3, 2018
@Azareal
Copy link
Contributor

Azareal commented Aug 4, 2018

SameSite cookies can probably mitigate this issue to some extent, although they're not available on mobile and older browsers, so it's good to have extra layers in.

@euantorano euantorano modified the milestones: 1.8.18, 1.8.19 Aug 12, 2018
@dvz dvz modified the milestones: 1.8.19, 1.8.20 Sep 4, 2018
@euantorano euantorano modified the milestones: 1.8.20, 1.8.21 Jan 5, 2019
@dvz dvz removed this from the 1.8.21 milestone Mar 12, 2019
@dvz dvz added s:feedback Status: Feedback. Further information/input needed and removed s:resolved Status: Resolved. Solution implemented or scheduled labels May 28, 2019
@Ben-MyBB Ben-MyBB added s:review-needed Status: Review Needed. Possible solution submitted and removed s:feedback Status: Feedback. Further information/input needed labels May 31, 2020
@Ben-MyBB Ben-MyBB added this to the 1.8.23 milestone Jun 7, 2020
euantorano pushed a commit that referenced this issue Jun 28, 2020
* Use verify_post_check() internally

* Add post code rotation & seed improvements

Co-authored-by: Tomasz Mlynski <devilshakerz@gmail.com>
@euantorano euantorano added s:resolved Status: Resolved. Solution implemented or scheduled and removed s:review-needed Status: Review Needed. Possible solution submitted labels Jun 28, 2020
@euantorano
Copy link
Member

Note: This change should be mentioned in the release post in case there are any plugins checking the post code manually.

lairdshaw pushed a commit to lairdshaw/mybb that referenced this issue Oct 11, 2021
* Use verify_post_check() internally

* Add post code rotation & seed improvements

Co-authored-by: Tomasz Mlynski <devilshakerz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:1.8 Branch: 1.8.x s:resolved Status: Resolved. Solution implemented or scheduled t:enhancement Type: Enhancement. Contains minor improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants