PHP get / post protection (wordpress)

Question

I have this function code to try and get some more protection for GET.

(e.g. index.php?name=eval(base64_decode(EVIL+CODE)) )

global $user_ID; 

    if($user_ID) {
                if(!current_user_can('administrator')) {
                        if (strlen($_SERVER['REQUEST_URI']) > 255 ||
                                stripos($_SERVER['REQUEST_URI'], "eval(") ||
                                stripos($_SERVER['REQUEST_URI'], "CONCAT") ||
                                stripos($_SERVER['REQUEST_URI'], "UNION+SELECT") ||
                                stripos($_SERVER['REQUEST_URI'], "base64")) {
                                        @header("HTTP/1.1 414 Request-URI Too Long");
                                        @header("Status: 414 Request-URI Too Long");
                                        @header("Connection: Close");
                                        @exit;
                        }
                }
        }

what can I add it to help protecting also POST ?(as much as possible - a total seal is not possible - i know )

The real question is: what are you trying to do? Protection from what? I mean you are not going to execute values coming from the client right? And please stop surpressing errors using `@` it's a really bad habit imo. — PeeHaa, Jun 11 '12 at 10:28
Just an honest side question: Where did you learn to use `global`? — PeeHaa, Jun 11 '12 at 10:30
@PeeHaa - point taken about the error suppressing . But it would be nice if you would help me understand how to do things, not only how NOT to do them ( I already know myself that my php skills are virtually non - existing :-) ) — Obmerk Kronen, Jun 11 '12 at 10:31
I am trying to help you. I believe I asked you some questions in my comment. — PeeHaa, Jun 11 '12 at 10:32
What am I trying to do ? Like I said in the question - I am trying to add another layer of protection on a fairly hackable wordpress system (I believe I am not the first to get a wordpress site hacked - and certainly not the last) - and the global $user_id is for wordpress.. — Obmerk Kronen, Jun 11 '12 at 10:35
Again: The real question is: what are you trying to do? Protection from what? I mean you are not going to execute values coming from the client right? Please try to be specific in what you are trying to achieve. — PeeHaa, Jun 11 '12 at 10:37
Your code will only protect when Its allowing code evaluation and sql injection. Its better to protect from the root. — Shiplu Mokaddim, Jun 11 '12 at 10:37
@shiplu.mokadd.im - yes . but sadly enough - wordpress itself (and some of it´s not security-savvy plugins) will do just that . and that is exactly my point - trying to add another layer . just in case . when you say from the root you mean in .htaccess ? — Obmerk Kronen, Jun 11 '12 at 10:51
@PeeHaa - Again : I am trying to add a new layer of protection from maliscious queries on wordpress system and plugins like described here : http://www.wpbeginner.com/plugins/protect-wordpress-against-malicious-url-requests/ and also here : http://wordpress.org/extend/plugins/block-bad-queries/ . I do not know how to be more clear on that . Maybe I just do not understand your question. (on top of being a bad coder :-) ) it is not my code , my question was simply how to add some layer for POST .. — Obmerk Kronen, Jun 11 '12 at 11:05
@ObmerkKronen Those posts you linked to are both pretty old (2010). What version of WP are you running? — PeeHaa, Jun 11 '12 at 19:44
@PeeHaa I know they are old. and I know some of the vulnerabilities had been fixed. I am running more than 200 sites, some of which are dated back to 2008 and use plugins or snippets. (like phpthumb (the old version)) that are not so secure. I am sorry, I do not see why this question is such a bad question - because of the bad approach/code , or because the idea of adding a new security layer for get/post is so bad ? if it is the code , so please someone teach me how to do that right , if the idea is so horrible - so please explain why. all I get up to now is down votes :-) — Obmerk Kronen, Jun 12 '12 at 01:46

score 1 · Accepted Answer · edited May 23 '17 at 12:29

Not really an answer, but it's a bit too much info for a comment.

I will write my answer here based on your last comment:

@PeeHaa I know they are old. and I know some of the vulnerabilities had been fixed. I am running more than 200 sites, some of which are dated back to 2008 and use plugins or snippets. (like phpthumb (the old version)) that are not so secure. I am sorry, I do not see why this question is such a bad question - because of the bad approach/code , or because the idea of adding a new security layer for get/post is so bad ? if it is the code , so please someone teach me how to do that right , if the idea is so horrible - so please explain why. all I get up to now is down votes :-)

That code is pretty horrible imo and should not be something you should use. Only in the comments you note the fact that you want to protect WP installations. Which is a pretty important piece of information missing in your question / from the tags.

Anyway as stated that code is pretty horrible, but since the fact I haven't ever used WP (for the issue of horrible code) I won't judge. But I will tell you what I think is bad about that code:

global $user_ID;

The use of the global keyword is a bad habit imho. Keep that in mind for new code when possible. Now for that if statement:

I don't have any clue what strlen($_SERVER['REQUEST_URI']) > 255 should prevent, but it may be the fact where WP or some plugin screws up at some point.

stripos($_SERVER['REQUEST_URI'], "eval(")

The above doesn't look just strange, because it would only be a problem when you would execute code coming from the client (which is something you would never do (unless you use some very wrong WP plugin)), but it is also just plain wrong. stripos() may also return 0 which will be falsy. SO that check may be useless. I don't know enough of wordpress and don't know whether something like this may be possible:

http://your-domain/eval(insert malicious code here)

REQUEST_URI always start with /. One last thing about those headers where the errors are surpressed. I suspect it prevent errors when the headers are already sent. What would happen when this is the case. Would the check still work.

The best thing you can do is upgrade WP and plugins or get rid of rotten plugins. That would be ideal. However as you have stated you have 200 installations which may make it impossible to do in the short term. So you may have to sort to indeed some code like this to "fix" any issues.

To get better answers in the future you might consider providing more detail in your question (e.g. the fact that you are trying to preotect WP installs). Also I think that would still be a very broad question. It is often better to ask a specific question.

Thanks for the answer and clarification . but I believe that the TITLE states "wordpress" . at any rate - I really DO appreciate the help I get here at stackexchange as a whole. If you will see my records, you will see that I also ANSWER quit a bit (only wordpress related). the issue is that sometimes - the more experianced users (like yourself) take for granted that novices (like me) have the same level of skills. :-) it is obviously not true in this specific case . your skills are clearly superior , but the only way for someone like me to learn is - well, asking. — Obmerk Kronen, Jun 12 '12 at 08:46
BTW - the GLOBAL thing is wp issue. sometimes you must use it (like `global $wpdb` when you need an acces to DB) — Obmerk Kronen, Jun 12 '12 at 08:48
@ObmerkKronen I suspected that it was a WP thing :) Just wanted to point out how you could improve your question / future question and add my 2 cents about the code. I hope you don't see it as a personal attack, because that is that what it is meant like. — PeeHaa, Jun 12 '12 at 08:53
I'd like to add that checking for `eval(` doesn't make any sense as the attacker could simply replace it with `eval (`. `base64` is just as pointless, because you could simply run some other function like `rot13` over it. Furthermore `eval` by far is not the only way to execute code. `preg_replace` and `assert` can also be used for that purpose. Additionally it's possible to execute code on the commandline using several functions. So, that's pretty much all pointless imho :) — NikiC, Jun 12 '12 at 10:18
@NikiC - leaving the specific bad coding aside and responding the arguments : I theoretically accept all your arguments . but they are a bit like saying that there is no point in locking your car because a real professional thief would know how to break in anyways. when you lock your door on your car (or house) you are providing some additional layer of protection against "amateurs" - the "pros" will get in anyways. but in a real world scenario - the "amateurs" are 90% of the thieves (attacks) . and the damage created is the same. so I bet you also lock your car. — Obmerk Kronen, Jun 13 '12 at 05:39

PHP get / post protection (wordpress)

1 Answers1