wjunkin / moodle-quiz_liveviewgrid

Dynamic quiz spreadsheet
2 stars 2 forks source link

Session expiration security issue #8

Closed danmarsden closed 5 years ago

danmarsden commented 5 years ago

When the report page is open, this plugin appears to keep the users session alive indefinitely which is generally understood as bad security practice. OWASP has some comments about this here: https://www.owasp.org/index.php/Session_Management_Cheat_Sheet#Session_Expiration

You could consider improving the way you perform the graphicshash check so that it doesn't result in the session remaining open forever. (probably by adding some form of timeout to stop refreshing the page after a certain time period has elapsed.)

If you do not want to change this prior to approval you should add some detail around this risk in your plugins db description on moodle.org and in your readme here on github.

wjunkin commented 5 years ago

I have modified the report.php code so that the report page only refreshes as long as there are new responses and the time from the last new response doesn't exceed the sessiontimeout value. I have done this by defining two css classes -- .blinkhidden and .blinking. I have put a warning right above the word "Responses" on the page and this warning is hidden. I obtain the value of sessiontimeout from the config table. I then count the number of graphicshash checks that are done and when this number*time per check exceeds the sessiontimeout value the page no longer does a graphicshash check and the warning becomes visible and is red and flashing. thus, the teacher is warned that the view of the responses is no longer live. Thanks, Dan, for pointing out this security vulnerability.