This bug is very difficult to reproduce under "real world" conditions, but we had this bug bite us on a production system and delete course data for over 100 courses. We had to recover the deleted course data from tape.

In mod/forum/lib.php, there is a sequence of calls when deleting a forum activity, which cleans up attachments:

forum_delete_instance() -> forum_delete_discussion() -> forum_delete_post() -> forum_delete_old_attachments()

The forum_delete_old_attachments function calls forum_file_area to get the path the the attachment directory, and then procedes to do an effective 'rm -rf <dir>'.

The bug is in the forum_file_area() function, this line:

return make_upload_directory( forum_file_area_name($post) );

There is no error checking on the return value of forum_file_area_name, which returns false if the parent discussion no longer exists in the database.

make_upload_directory(false) returns the moodledata directory, so the attachment cleanup code will remove all moodledata!

This happened on a production instance that we host, because two simultaneous deletes of the same course were happening at the same time. It has since been very difficult to reproduce that situation, however here are some steps to demonstrate the bug by manually changing some values in the database.

- Make a new forum
- Post to it
- Reply to that post with another post containing an attachment
- Now in the database, update the record for the 2nd post and set discussion = 0 (this gets the forum_file_area_name function to return false).
- Delete the forum or whole course, and watch as your moodledata directory gets entirely erased.

In the real world, the bug occurs only if the parent discussion record gets deleted before the forum_file_area_name() function executes (eg. multiple simultaneous deletes are happening).

I've attached a patch which does some error checking in the forum_file_area() function to fix the problem.

Although this bug is an extremely rare occurence due to the intricate timing involved, it is definately unsafe to assume forum_file_area_name() will not return false, considering it has two "return false" statements in it.