I Wrote Some Really Dangerous Code

Sometimes when looking back at something I wrote in the past, perhaps two years ago, I discover some really terrible code I’ve written. Some of it is simply poor quality with really bad formatting, and some of it is outright dangerous from a security point. This evening I discovered some of that “dangerous” code inside of a plugin I wrote about two years ago; a plugin that has been used very, very heavily on a site that has hundreds of thousands of page views every day. I want to show you what the issue was, why it was dangerous, and how I fixed it.

Note: the plugin described here is NOT available to the public so no one’s site is at risk.

The plugin is similar to User Submitted Image Gallery tutorial series I wrote (it’s the plugin the tutorial was based on), but includes several components that were never included in that tutorial series, primarily the ability for a user to edit / delete their own images after having them submitted to the gallery.

It was in this edit / delete capability that I found a major, major, mission critical security flaw.

First let me walk you through how I discovered that there was a problem in the first place.

Over the last month or so, we (the site owners and I) noticed that a large number of spam posts were getting created on the main site of the WordPress multisite install. All posts were getting saved as drafts so there was no immediate threat to destroying the content of the site, but there were 5 – 10 posts get created every day. This was alarming for obvious reasons.

The site has over 70,000 registered users so we first assumed that the posts were coming from a registered account on the site, perhaps one that had somehow gotten promoted to a contributor role, but this theory was quickly dashed because the only users that had any write permissions on the site were all very trusted authors, editors, and admins. No contributors at all.

The second theory we had was that perhaps there was a vulnerability in XML-RPC or Atom published, or a user had somehow gotten authenticated with the site, and posts were coming in this way. This seemed logical, but we quickly discarded it as well after we disabled both services entirely and the draft posts were still getting created.

Our third theory was that there was a vulnerability in one of the plugins or in the highly function based theme the site was using. The theme itself is enormous (50+ files and thousands and thousands of lines of code) and there were around 50 plugins used across the network, most of which were instrumental to the site functioning. Finding a vulnerability in this many files was a daunting task, to say the least.

The fourth theory was that the site had gotten infected with malware that was opening up a hole to the spammers. We used a couple of exploit scanners to look for issues, but none of them, including the awesome Sucuri scanner, found even a trace of an infection (relieving).

Once I discovered the issue, it turned out to be our third theory that was correct: there was a vulnerability in one of the plugins (the gallery plugin mentioned above). So how did I find the vulnerability?

Tracking the vulnerability down actually turned out to be quite simple. After a suggestion from Justin Sainton, I decided to write up a quick logging plugin that would send me reports on a variety of data each time a post was saved (created, published, updated). Knowing that every time a post goes through a status transition, the “save_post” action is fired, this was pretty simple to do. I wrote up a quick plugin and dropped it in mu-plugins:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22

<?php
/*
Plugin Name: Post Save Log
Description: A plugin to help track down the draft post spammer
Version: 1.0
Author: Pippin Williamson
Author URI: https://pippinsplugins.com
Contributors: mordauk
*/
 
function pw_log_post_save( $post_id ) {
 
	$log = "== POST ID: " . $post_id . " ==\n\n";
	$log .= "== REQUEST DATA ==\n\n";
	$log .= print_r( $_REQUEST, true );
	$log .= "\n\n==SERVER DATA ==\n\n";
	$log .= print_r( $_SERVER, true );
 
	wp_mail( 'myemail@domain.com', 'Post Save Log', $log );
 
}
add_action( 'save_post', 'pw_log_post_save' );

What this did was simply send me an email with post ID that was saved, the contents of the $_REQUEST and $_SERVER super globals printed out. The $_REQUEST and $_SERVER vars are arrays containing any and all POST/GET data and also a lot of information about the person / computer making the request.

What did this yield me? Well pure gold, to put it one way. Within 30 minutes of adding in my little plugin, I had a nice little report that looked like this:

== POST ID: 41621 ==
 
== REQUEST DATA ==
 
Array
(
    [image-updated] => 1
    [pig-image-action] => edit
    [pig-image-id] =>
    [pig-subsite-id] =>
    [pig-subsite-image-id] =>
    [pig-referrer] => http://SOMESITEDOMAIN.com/dashboard/
    [pig-image-title] => Jerseys Big Size56-60
 Give You A Surprise
 
    [pig-image-desc] => <a href=\"http://www.monclerveste2u-fr.com/moncler-enfants/1064-moncler-enfants-bleu\">Moncler Pas Cher Avis</a>  , aS4toQdTOt
<a href=\"http://www.monclerveste2u-fr.com/moncler-enfants/1065-moncler-enfants-all-black\">Ou Acheter Moncler Pas Cher</a>  , BsYT1uSb3xK
<a href=\"http://www.monclerveste2u-fr.com/moncler-enfants/1066-moncler-enfants-vestes-noir\">Doudoune Moncler Femme Pas Cher Neuf</a>  , dEldC1kGJn
<a href=\"http://www.monclerveste2u-fr.com/moncler-enfants/1067-moncler-enfants-bleu\">Manteau Moncler Femme Pas Cher</a>  , QxO3CrRslB
<a href=\"http://www.monclerveste2u-fr.com/moncler-enfants/1068-moncler-vestes-bleu-noir\">Moncler Ralph Lauren Pas Cher</a>  , sCydHrBYi
<a href=\"http://www.monclerveste2u-fr.com/moncler-enfants/1069-moncler-enfants-bleu-fonce\">Je Porte Pas De Moncler C\'est Pas Assez Cher</a>  , IaOEbRgqA
<a href=\"http://www.monclerveste2u-fr.com/moncler-enfants/1070-moncler-vestes-rose\">Doudoune Imitation Moncler Pas Cher</a>  , mUyq7FzGDe
<a href=\"http://www.monclerveste2u-fr.com/moncler-enfants/1071-moncler-enfants-rouge\">Doudoune Moncler Pas Cher Homme</a>  , JsKThEcwV.
    [submit] => Submit Update
)
 
==SERVER DATA ==
 
Array
(
    [SERVER_SOFTWARE] => Apache/2.2
    [REQUEST_URI] => /dashboard/?image-updated=1
    [REDIRECT_UNIQUE_ID] => UKwNmwq7@JwAAE2GMnkAAAAL
    [REDIRECT_SCRIPT_URL] => /dashboard/
    [REDIRECT_SCRIPT_URI] => http://SOMESITEDOMAIN.com/dashboard/
    [REDIRECT_PHP_DOCUMENT_ROOT] => /mnt/target05/346407/346408/www.SOMESITEDOMAIN.com/web/content
    [REDIRECT_STATUS] => 200
    [UNIQUE_ID] => UKwNmwq7@JwAAE2GMnkAAAAL
    [SCRIPT_URL] => /dashboard/
    [SCRIPT_URI] => http://SOMESITEDOMAIN.com/dashboard/
    [PHP_DOCUMENT_ROOT] => /mnt/target05/346407/346408/www.SOMESITEDOMAIN.com/web/content
    [HTTP_USER_AGENT] => Mozilla/5.0 (Windows NT 6.1) AppleWebKit/536.5 (KHTML, like Gecko) Chrome/19.0.1084.56 Safari/536.5
    [HTTP_X_FORWARDED_FOR] => xxx.xxx.xx.xxx
    [HTTP_ACCEPT] => */*
    [CONTENT_TYPE] => application/x-www-form-urlencoded
    [HTTP_HOST] => SOMESITEDOMAIN.com
    [HTTP_REFERER] => http://SOMESITEDOMAIN.com/dashboard/?image-updated=1#gallery_tab
    [HTTP_X_MOSSO_DT] => PHP5-7 Pool
    [HTTP_PRAGMA] => no-cache
    [HTTP_X_CLUSTER_CLIENT_IP] => xxx.xxx.xx.xxx
    [HTTP_COOKIE] => X-Mapping-iikfnbbj=46E350C951A81595566AD230023D7046; PHPSESSID=nuvoubu1nbivpatrg2bvoppap7
    [CONTENT_LENGTH] => 1543
    [PATH] => /sbin:/usr/sbin:/bin:/usr/bin
    [SERVER_SIGNATURE] => <address>Apache/2.2 Server at SOMESITEDOMAIN.com Port 80</address>
 
    [SERVER_NAME] => SOMESITEDOMAIN.com
    [SERVER_ADDR] => 10.187.248.156
    [SERVER_PORT] => 80
    [SERVER_ADMIN] => root@localhost
    [SCRIPT_FILENAME] => /mnt/target05/346407/346408/www.SOMESITEDOMAIN.com/web/content/index.php
    [REMOTE_PORT] => 10680
    [REDIRECT_QUERY_STRING] => image-updated=1
    [REDIRECT_URL] => /dashboard/
    [GATEWAY_INTERFACE] => CGI/1.1
    [SERVER_PROTOCOL] => HTTP/1.0
    [REQUEST_METHOD] => POST
    [QUERY_STRING] => image-updated=1
    [SCRIPT_NAME] => /index.php
    [DOCUMENT_ROOT] => /mnt/target05/346407/346408/www.SOMESITEDOMAIN.com/web/content
    [REMOTE_ADDR] => 174.139.58.234
    [PHP_SELF] => /index.php
    [REQUEST_TIME] => 1353452955
    [argv] => Array
        (
            [0] => image-updated=1
        )
 
    [argc] => 1
    [HTTPS] =>
)

If you look at the SERVER DATA section, you’ll see that the IP address of the computer this was coming from was included. Awesome, that means I can track this spammer down. But wait! Even better, if you look in the REQUEST DATA section, you’ll see this: pig-image-action] => edit. Why is this valuable? It was priceless because I knew exactly where that came from: I wrote the plugin that used that unique action name.

This told me everything I needed to know to find the vulnerability, so I opened up the file and knew immediately that I’d solved it. Here’s the exact contents of the file when I opened it:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110

<?php
 
/************************************
* Process Image Edits and Deletions
************************************/
 
function pig_process_image_edit() {
 
	global $blog_id;
	$data = (!empty($_POST)) ? true : false;
	if($data) // if data is being sent
	{	
		if(isset($_POST['pig-image-action']) && $_POST['pig-image-action'] == 'edit') {
 
			// edit an image
			$image_id 		= strip_tags(stripslashes($_POST['pig-image-id'])); 		// get the image ID on the main site
			$site_id 		= strip_tags(stripslashes($_POST['pig-subsite-id'])); 		// get the ID of the site the image belongs to
			$subsite_image_id 	= strip_tags($_POST['pig-subsite-image-id']); 			// get the ID of the image on the subsite
			$name	 		= strip_tags(stripslashes($_POST['pig-image-title'])); 		// get the name of the image
			$desc			= strip_tags(stripslashes($_POST['pig-image-desc'])); 		// get the image description
			$url			= strip_tags(stripslashes($_POST['pig-referrer'])); 		// get the redirect URL
			$error			= NULL;
			if(!$name || $name == '')
			{
				$error .= 'Please enter a name for the image.<br/>';
			}
			if(!$desc || $desc == 'Describe your image')
			{
				$error .= 'Please enter a description.<br/>';
			}
			$mature = isset($_POST['pig-image-mature']) ? 'on' : false;
 
 
			// everything ok
			if(!$error)
			{
				// update the image on the main site
				$updated_image_id = wp_update_post(array(
						'ID'		=> $image_id,
						'post_title'	=> $name,
						'post_content'	=> $desc
					)
				);
				update_post_meta($image_id, 'pig_mature', $mature);
				switch_to_blog($site_id);
					// update the image on the sub site
					$updated_sub_site_image_id = wp_update_post(array(
							'ID'		=> $subsite_image_id,
							'post_title'	=> $name,
							'post_content'	=> $desc
						)
					);
					update_post_meta($subsite_image_id, 'pig_mature', $mature);
				restore_current_blog();
 
				// the IMAGE post was created okay
				if($updated_image_id && $updated_sub_site_image_id)
				{			
					wp_redirect($url . '?image-updated=1#gallery_tab');	exit;		
				}
				else
				{
					wp_redirect($url . $url . '?image-updated=0#gallery_tab'); exit;
				}
			}
			// if there's an error
			else
			{
				header ("Location: " . $url . '?image-updated=0&fields-empty=1#gallery_tab');
			}
		} else if(isset($_POST['pig-image-action']) && $_POST['pig-image-action'] == 'delete') {
			// delete an image
			$image_id 		= strip_tags(stripslashes($_POST['pig-image-id'])); 		// get the image ID on the main site
			$site_id 		= strip_tags(stripslashes($_POST['pig-subsite-id'])); 		// get the ID of the site the image belongs to
			$subsite_image_id 	= strip_tags($_POST['pig-subsite-image-id']); 			// get the ID of the image on the subsite
			$url			= strip_tags(stripslashes($_POST['pig-referrer'])); 		// get the redirect URL
			$error			= NULL;
 
			if(!$image_id)
			{
				$error .= 'Something went wrong.<br/>';
			}
			if(!$subsite_image_id)
			{
				$error .= 'Something went wrong.<br/>';
			}
 
			// everything ok
			if(!$error)
			{
				// remove the image from the main site
				wp_delete_post($image_id);
 
				switch_to_blog($site_id);
					// remove the image on the sub site
					wp_delete_post($subsite_image_id);
				restore_current_blog();
 
				header ("Location: " . $url . '?image-removed=1#gallery_tab');			
 
			}
			// if there's an error
			else
			{
				header ("Location: " . $url . '?image-removed=0#gallery_tab');
			}
		}
	}
}
add_action('init', 'pig_process_image_edit');

Do you see the problem(s)? There are several of them. The first problem is that the function contains no permission checks at all. NONE! There is not a single check to see if the current user owns the image being edited; there is no check to see if the image being edited actually exists; and there’s not even a check to see if the user is logged in!

Whoops!

Due to the lack of permission checks, and the lack of a check to see if the image ID passed in $_POST[‘pig-image-id’] actually existed, this allowed the spammer to perform a remote request to the site and create a new draft post with every request.

When wp_update_post() is given a post ID that doesn’t exist (or not given an ID), it simply creates a new post and sets its status to “draft”, assuming no post_status parameter is given.

Because there wasn’t a single check to see if the user making the request was logged-in, and also no check to see if the current user was the owner of the image, it was possible for ANY user (even those logged-out) to update the images of any other user, as well as create new ones (though only as drafts).

This was really bad.

But wait, it gets worse. Much worse.

Take a look at this piece of the function:

} else if(isset($_POST['pig-image-action']) && $_POST['pig-image-action'] == 'delete') {
	// delete an image
	$image_id 		= strip_tags(stripslashes($_POST['pig-image-id'])); 		// get the image ID on the main site
	$site_id 		= strip_tags(stripslashes($_POST['pig-subsite-id'])); 		// get the ID of the site the image belongs to
	$subsite_image_id 	= strip_tags($_POST['pig-subsite-image-id']); 			// get the ID of the image on the subsite
	$url			= strip_tags(stripslashes($_POST['pig-referrer'])); 		// get the redirect URL
	$error			= NULL;
 
	if(!$image_id)
	{
		$error .= 'Something went wrong.<br/>';
	}
	if(!$subsite_image_id)
	{
		$error .= 'Something went wrong.<br/>';
	}
 
	// everything ok
	if(!$error)
	{
		// remove the image from the main site
		wp_delete_post($image_id);
 
		switch_to_blog($site_id);
			// remove the image on the sub site
			wp_delete_post($subsite_image_id);
		restore_current_blog();
 
		header ("Location: " . $url . '?image-removed=1#gallery_tab');			
 
	}
	// if there's an error
	else
	{
		header ("Location: " . $url . '?image-removed=0#gallery_tab');
	}
}

You will see that there also weren’t any permission checks in place for when a delete image request was sent, meaning that ANYONE could have deleted ANY piece of content on the entire site. The only thing they would have needed was the ID of the post, attachment, page, or custom post type they wanted to delete. This could have been detrimental!

It was extremely fortunate that no one discovered this severe vulnerability.

How were these issues fixed?

Fixing these problems was actually really simple. All I needed to do was add in a couple of checks:

  • make sure the user is logged-in
  • ensure the current user is the owner (post_author) of the image
  • check that the image being updated or deleted actually exists before processing the request

Here is the updated file:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124

<?php
 
/************************************
* Process Image Edits and Deletions
************************************/
 
function pig_process_image_edit() {
 
	global $blog_id;
	if( ! empty( $_POST ) ) {
		if ( isset( $_POST['pig-image-action'] ) && $_POST['pig-image-action'] == 'edit' ) {
 
			if ( !is_user_logged_in() )
				return;
 
			// edit an image
			$image_id    		= strip_tags( stripslashes( $_POST['pig-image-id'] ) );   // get the image ID on the main site
			$site_id    		= strip_tags( stripslashes( $_POST['pig-subsite-id'] ) );   // get the ID of the site the image belongs to
			$subsite_image_id  	= strip_tags( $_POST['pig-subsite-image-id'] );     // get the ID of the image on the subsite
			$name     		= strip_tags( stripslashes( $_POST['pig-image-title'] ) );   // get the name of the image
			$desc    		= strip_tags( stripslashes( $_POST['pig-image-desc'] ) );   // get the image description
			$url    		= strip_tags( stripslashes( $_POST['pig-referrer'] ) );   // get the redirect URL
			$error    		= NULL;
 
			if ( get_current_user_id() != get_post_field( 'post_author', $image_id ) )
				wp_die( 'You do not have permission to edit this image.', 'Error' );
 
			$image = get_post( $image_id );
 
			if ( empty( $image ) )
				return;
 
			if ( !$name || $name == '' ) {
				$error .= 'Please enter a name for the image.<br/>';
			}
			if ( !$desc || $desc == 'Describe your image' ) {
				$error .= 'Please enter a description.<br/>';
			}
 
			$mature = isset( $_POST['pig-image-mature'] ) ? 'on' : false;
 
			// everything ok
			if ( ! $error ) {
 
				// update the image on the main site
				$updated_image_id = wp_update_post( array(
						'ID'   => $image_id,
						'post_title' => $name,
						'post_content' => $desc
					)
				);
 
				update_post_meta( $image_id, 'pig_mature', $mature );
 
				switch_to_blog( $site_id );
 
				// update the image on the sub site
				$updated_sub_site_image_id = wp_update_post( array(
						'ID'   => $subsite_image_id,
						'post_title' => $name,
						'post_content' => $desc
					)
				);
 
				update_post_meta( $subsite_image_id, 'pig_mature', $mature );
 
				restore_current_blog();
 
				// the IMAGE post was created okay
				if ( $updated_image_id && $updated_sub_site_image_id ) {
					wp_redirect( $url . '?image-updated=1#gallery_tab' ); exit;
				} else {
					wp_redirect( $url . $url . '?image-updated=0#gallery_tab' ); exit;
				}
			} else {
				// if there's an error
				header( "Location: " . $url . '?image-updated=0&fields-empty=1#gallery_tab' );
			}
		} else if ( isset( $_POST['pig-image-action'] ) && $_POST['pig-image-action'] == 'delete' ) {
			// delete an image
			$image_id    		= strip_tags( stripslashes( $_POST['pig-image-id'] ) );   // get the image ID on the main site
			$site_id    		= strip_tags( stripslashes( $_POST['pig-subsite-id'] ) );   // get the ID of the site the image belongs to
			$subsite_image_id  	= strip_tags( $_POST['pig-subsite-image-id'] );     // get the ID of the image on the subsite
			$url    		= strip_tags( stripslashes( $_POST['pig-referrer'] ) );   // get the redirect URL
			$error    		= NULL;
 
			if ( get_current_user_id() != get_post_field( 'post_author', $image_id ) )
				wp_die( 'You do not have permission to edit this image.', 'Error' );
 
			$image = get_post( $image_id );
 
			if ( empty( $image ) )
				return;
 
			if ( !$image_id ) {
				$error .= 'Something went wrong.<br/>';
			}
			if ( !$subsite_image_id ) {
				$error .= 'Something went wrong.<br/>';
			}
 
			// everything ok
			if ( !$error ) {
 
				// remove the image from the main site
				wp_delete_post( $image_id );
 
				switch_to_blog( $site_id );
 
				// remove the image on the sub site
				wp_delete_post( $subsite_image_id );
 
				restore_current_blog();
 
				header( "Location: " . $url . '?image-removed=1#gallery_tab' );
 
			} else {
				// if there's an error
				header( "Location: " . $url . '?image-removed=0#gallery_tab' );
			}
		}
	}
}
add_action( 'init', 'pig_process_image_edit' );

The changes needed to secure the function were actually pretty minimal and only took me about 5 minutes, but it should be clear that the new checks added fix a very, very serious problem.

What does this teach us? Several things:

Always make sure the user doing things has permission to do it. No exceptions.

Everyone writes bad code. Whether you have been programming for 1 year or 20 years, every single developer has written (or will) bad code that has major problems. Hopefully our bad code doesn’t cause anyone significant harm or monetary loss; in this case I got really lucky and got aware with it relatively unscathed, and so did the company I work with.

We learn from our mistakes and get better every day, and hopefully we’re able to educate others with our own mistakes.

23 thoughts so far

Have your say

  1. Tony Perez

    Wow, that’s a gem of a find. Very nice write-up. Very very good thing that was not publicly available, that would have caused some serious problems if it was widely adopted. Really glad though that you not only identified but shared the experience. I can see this being rolled into a presentation.

    if I’m understanding correctly you were looking at a number of issues that range from role escalation, authentication control issue, possible remote file and local file inclusions and huge potential for XSS attacks.

    • Pippin

      Yeah, so glad this wasn’t in a public plugin. It was work I did custom just for the client (whew).

      Yes, there were a number of issues, though remote file inclusion was one of them, simply because we didn’t allow the image files themselves to be edited (or replaced).

  2. Syed Balkhi

    Great read Pippin. Always love learning from other folk’s mistakes. Thanks for showing us exactly how you tracked the issue. I might use this script in the future if needed 🙂

  3. Japh

    Fantastic write-up, Pippin! It’s easy to forget, that sometimes it can be just as useful to demonstrate where something went wrong, and how not to do things, even better if the correction is included too.

    • Pippin

      That’s so true. It’s definitely easier to grasp the concept of what NOT to do when one is presented with said bad example and the solution at the same time.

  4. Chris Lema

    Pippin,
    Great write up! The details and honesty are fantastic, as is the highlight that everyone writes code that they later look back on and wonder, “what the hell was I doing?”. Thanks!

  5. Tom McFarlin

    Killer work, Pippin – on both tracking the bug and showing your process in this post.

  6. JeffM

    Interesting mea culpa right there, Pippin. And if confession is good for the soul, you must be pretty-well buzzing right now.

    My own habit of years has been to start every included file with

    if ( ! defined( 'ABSPATH' ) ) die();

    Kills 99% of household germs, doncha know. But as you point out, there’s really not much diference in effort between doing it right and getting it catastrophically wrong.

    • Pippin

      Checking for ABSPATH is a great way of adding an extra layer.

    • JeffM

      Every WordPress plugin file is capable of defending itself. It’s just a matter of doing it. On line 1.

    • Keith

      JeffM, first thanks for the tip, but could you explain more about what that one line of code does and how it protects?

      Pippin, excellent write up on a before and after, great explanation!

    • Pippin

      Keith that line helps protect from external requests trying to force things to happen. With the problem that I had, everything was being externally, meaning the attacker wasn’t even inside of WordPress. The ABSPATH constant will only ever be defined if inside of WordPress, so checking that it is defined immediately prevents any external attempts.

    • Luis Alejandre

      Great tip, Jeff!
      Using that line of code seems as simple as perfect.

    • JeffM

      All the tools are there in WordPress, or in PHP itself.
      In the current climate, I’d say it’s no more than due diligence. IANAL, but it’s the least any client deserves.

    • Hajar Rinjani Trekking

      Hello Jeff
      Thanks for the useful info especially for us who don’t really understand code.

  7. Elliott

    Great read! Nice to know the technique you used to find the bug.

  8. Brad

    You nailed it. “Everybody writes bad code.” Which is one of many good reasons why everyone should ensure they have incremental backups of their sites. Even if that delete vulnerability had been exploited, backups would save the day.

  9. Luis Alejandre

    Admitting one´s own mistakes is”rara avis” nowadays, Pippin. And then you explained its details thoroughly. So we should thank you twice. Great article!

  10. Maor Chasen

    Great job! Happens to the best of us!

  11. srcnix

    Nice article, it’s good to see that there are people still out there that help others learn to get in a bug tracking frame of mind. Thanks for sharing the experience fella.

    Just my two pence though, from a logical programmatic point of view it would be best to check if the user is logged in straight after you check that the request method is POST and also, include the checking if the post key is set. Doing so will lower processing time in the delete routine as currently you are processing four vars before you check a permission. I would have thought the user needs to be logged in, in order to delete, which should ideally be checked prior to checking if the user owns the image.

    • Pippin

      Thanks for the feedback, you’re absolutely right 🙂

    • srcnix

      No problem. Always happy to help!

  12. Rommel

    thanks for share the technique you used to find the bug!! really useful

Comments are closed.

Leave a reply

Join the discussion