Alright - after looking through the code (it's been a while since I wrote this/thought about it), I have some questions - because this still looks secure to me. Well - I should point out: The page should NOT be accessible without authentication, I totally agree.
However, I'm not sure my nonce implementation is wrong, or that anything can be done from accessing that page outside of wp-admin. I don't use check_admin_referer(), but I do use wp_verify_nonce(), on the code where the actual actions are performed. If the nonce doesnt match, no action is performed (assuming I'm using wp_verify_nonce() right, but I believe I am). This post:
recommends using check_admin_referer(), as you do. However - this post:
suggests wp_verify_nonce - based on the style of my code, I'm guessing I pulled it straight from that post.
Further - the processing code is not in that file (cg-tvs-admin-panel-display.php). That file really just holds display code - the file that does do the processing is class-cg-tvs-plugin.php at line 253. At line 255, before any processing happens, we check if the user has permission to manage options. After that, on lines 272, 283, and 295, you can see where those nonces get checked, based on what the user is trying to do.
Again - I don't mean to imply there's no problem here - there is, and I want to fix it - but I want to make sure I know what was wrong and why it was wrong, so I can make sure it gets fixed properly.
A couple of other things:
- I'll throw in a blank index.php file to prevent directory listing. I should have done this already
- The only reason the page that loads at all is because there's very little interaction with wordpress - it's almost all display code (which I mostly try to keep separate from the processing code). However, this makes me realize: All the files in teh plugin *can* be loaded externally, they just fail because they're trying to interact with WordPress. Is it best practice to include code on *every* plugin file (well, php file) to prevent direct loading? This situation makes me think it is, but I'd love some further input.
I think that covers it. Again - based on my understanding of nonces, and the fact that processing code is stored in another file, which has a permissions check, I don't think there's the potential for malicious intent here - but I want to make sure I've got my ducks in a row before I release an update. Regardless, I'll put out an update with the aforementioned changes tomorrow morning - I'd just rather make sure it's right than have to make 2 releases.