Limiting Output from Unmaintainable PHP Code

[Borek]

I have a site made in PHP eons ago by someone I no longer have contact with. Lately site was attacked with a SQL injection attempt. Input is sanitized, so there is no immediate danger for the database, but this attack exposed vulnerability - if value of one the parameters is not from the predefined set, program ends in an endless loop, throwing warnings. That's in a way equivalent to a case statement missing default. In effect code generates multimegabyte output, eating bandwidth (20 GB on March 10th - which is what caught my attention, as typically daily traffic it is in tens of MB range). It doesn't happen often, still, judging from logs every few months someone tries hacking.

I did some digging in the source and I don't see how to correct the problem without writing everything from scratch - perfect example of unmaintainable code. Site is not worth the effort, still, it brings enough money from adsense to pay a third of the server costs, so I don't feel like just closing it.

Any ideas about how to limit the output without changing the code? Or with just a small changes? So far I thought about limiting execution time through max_execution_time in php.ini or by calling set_time_limit(1), but perhaps you can think of some other, better ways?

 

[AlephZero]

Can you add an error handler to treat the warnings as errors and terminate the program? Something like
http://stackoverflow.com/questions/2071024/treating-warnings-as-errors

I'm assuming that adding new code is easier than fixing the old code!

 

[.Scott]

You can count how many time a particular output is executed - and end it if it runs past 1000 or whatever.
You can look at the IP addresses of the hacker and the next time offer them some alternative content.

 

[Borek]

Can you add an error handler to treat the warnings as errors and terminate the program? Something like
http://stackoverflow.com/questions/2071024/treating-warnings-as-errors

Yes, error handler crossed my mind as well.

You can count how many time a particular output is executed - and end it if it runs past 1000 or whatever.

Can you elaborate? How to count?

 

[.Scott]

Obviously, I don't know what your starting point is.

I'll assume that you have several "main" php files that are intended to be entered directly and many others that are only intended to be "include"d from other php files. That being the case, you should put this kind of statement as the first statement in each main php file:
define ('BOREK_APPROVED_ENTRY', true);

And this statement at the start of all the *.inc files and other php files:
if ( !defined('IN_CERTMESSAGEBOARD') )
{
die("Hacking attempt - common");
}

This is a very common anti-hacking method - and there's a good chance you already have this in your code.

I don't know what your economy of effort is. I describe this without using an include file, but this new code can obviously be included in one.

This new code sets "$VolumeHackLimit" to 1000 and then provides a function to allow it to be counted down. It goes only at the top of the php files that are intended for webpage entry. Elsewhere in the main php file or in any included code, you make a call to this new function - as shown here:

<?php
define ('BOREK_APPROVED_ENTRY', true);

$VolumeHackLimit = 1000;
function volume_hack_check()
{
  global $VolumeHackLimit;
  if($VolumeHackLimit<=0) die("Bandwidth exceeded!");
  $VolumeHackLimit--;
}

... later in your code ...

  volume_hack_check();

...

?>

 

[.Scott]

I just glanced at this post again and noticed a mistake. I copied some code without changing it to match this dialog. My apologies if this caused you delay. Here it is with the correction - since it's too late to edit the original:

I'll assume that you have several "main" php files that are intended to be entered directly and many others that are only intended to be "include"d from other php files. That being the case, you should put this kind of statement as the first statement in each main php file:
define ('BOREK_APPROVED_ENTRY', true);

And this statement at the start of all the *.inc files and other php files:
if ( !defined('BOREK_APPROVED_ENTRY') )
{
die("Hacking attempt - common");
}

This is a very common anti-hacking method - and there's a good chance you already have this in your code.

I don't know what your economy of effort is. I describe this without using an include file, but this new code can obviously be included in one.

This new code sets "$VolumeHackLimit" to 1000 and then provides a function to allow it to be counted down. It goes only at the top of the php files that are intended for webpage entry. Elsewhere in the main php file or in any included code, you make a call to this new function - as shown here:

<?php
define ('BOREK_APPROVED_ENTRY', true);

$VolumeHackLimit = 1000;
function volume_hack_check()
{
  global $VolumeHackLimit;
  if($VolumeHackLimit<=0) die("Bandwidth exceeded!");
  $VolumeHackLimit--;
}

... later in your code ...

  volume_hack_check();

...

?>

 

[Borek]

If I understand your idea correctly, I would have to browse the code to identify all the places where I should put volume_hack_check() call. That's exactly what I want to avoid.

 

[.Scott]

If I understand your idea correctly, I would have to browse the code to identify all the places where I should put volume_hack_check() call. That's exactly what I want to avoid.

Ahhh. So you can't tell what output is being generated? And besides, the problem might be with a single template.

You've already mentioned "max_execution_time" in the PHP runtime configuration. Perhaps the function "set_time_limit(1);" can be used more surgically. If you decide to use these, you also need to consider the "implicit_flush" parameter. Setting that parameter to TRUE could make your site too sluggish, but if it doesn't, it will tie the execution time (or rather the period of execution) to the amount of output generated.

Actually, if you've found the request in the log, you do know where the output is coming from. All you have to do is issue the request from your own browser and see what you get back. Since its megabytes, there should be terms that repeat thousands of times. Those are the terms that you would "volume_hack_check()". Of course, it would be best to do this after setting those PHP.ini parameters first - even if only for debugging.

You said you found the bad requests in your sites logs? So you could check $_SERVER['REMOTE_ADDR'] on entry to abandon or side-track requests from those IPs. Apparently the malformed requests are not easy to recognize, otherwise you would just check them on entry and bounce the request.

In any case, good luck.