• Unpack bugs (memory allocation problems)

Posted: **Thu Sep 20, 2007 3:37 am**

Seems it incorrectly handles many "corner cases". Some of them cause thread exceptions which include bad allocations and excessive memory usage peaks. If you have AUX listener enabled, you are open to DOS attacks (btw, why on earth AUX uses packed format, not raw text data?).

Code: Select all

packed := "S-1"; print(packed + " -> " + unpack(packed));

S-1 -> error{ errortext = "Unable to unpack string length" }
Exception in: scripts/console/about.ecl in: : basic_string

Code: Select all

packed := "S-1:"; print(packed + " -> " + unpack(packed));

Exception in: scripts/console/about.ecl in: : basic_string

On a system with 2GB of ram, POL uses ~100MB, I ran this:

Code: Select all

packed := "S1000000000:"; print(packed + " -> " + unpack(packed));

Exception in: scripts/console/about.ecl in: : bad allocation

Server froze for a while, mem usage peaked to 1GB, finally an exception was thrown ant it returned to normal.

But hey, why stop on strings? Let's mess with arrays!
I've created a basic AUX listener that just printed out on the console everything it received:

Code: Select all

use basic;
use os;

program aux(conn)
	var event;
	
	SysLog("[AUX] connection initiated: "+conn);
	
	while (conn)
		event := wait_for_event(5);
		if (!event)
			continue;
		endif

		event := event.value;
		if (event == error)
			SysLog("[AUX] bad data: "+event.errortext);
			continue;
		endif
		
		print(event);
	endwhile
	
	SysLog("[AUX] connection closed: "+conn);
endprogram

Then I connected with raw telnet application (PuTTY) and send the following:

Code: Select all

Hello world!
sHello world!
S12:Hello world!
S-1:
a100000000:a1000000:a10000000:

Results?

Code: Select all

Starting Aux Listener (:auxremote:aux_remote, port 5557)
syslog [pkg/auxsvc/aux_remote.ecl]: [AUX] connection initiated: <AuxConnection>
syslog [pkg/auxsvc/aux_remote.ecl]: [AUX] bad data: Unknown object type 'H'
Hello world!
Hello world!
Thread exception: basic_string
syslog [pkg/auxsvc/aux_remote.ecl]: [AUX] connection initiated: <AuxConnection>
Thread exception: bad allocation
Commands:
  a: Test
  S: Lock/Unlock console
  ?: Help (This list)
Console is now unlocked.
Command aborted due to: bad allocation
Command aborted due to: bad allocation
Thread exception: bad allocation
Thread exception: bad allocation

Memory usage went up to 1.7GB and stood there. No subsequent AUX connections were accepted, no console commands either: no scripts were able to load. That effectively means the server is dead:

Code: Select all

  scin: 0  scsl: 0  MOB: 0  TLI: 0
  scin: 0  scsl: 0  MOB: 0  TLI: 0

Fun, eh?

Posted: **Tue Oct 02, 2007 6:14 am**

Because you are trying to unpack BAD data, simple enough. Unless you can ensure the packed data is correct, do NOT use unpack on it.

Two ways to ensure it is correct. It had pack() used on it, or, if this is for aux for example (with php), use the PHP tools from distro SVN that have the pack and unpack features for PHP.

Now, to answer the question, why does AUX xmit packed data..... because it is best to help transmit the data as it appears in POL. Again, the PHP library handles array, etc etc. for unpacking and packing in PHP. No clue if anyone released a C++ library for this or not.

Now, you want to prevent against denial of service..... set up IP rules for the ports using firewall software/hardware to protect those ports. Easy enough.

Posted: **Tue Oct 02, 2007 6:41 am**

Also, for your reference, this is the expected and required formatting for packed data. The first set is how it works, small s for straight string, large S for a string with the length defined after S and so on:

Code: Select all

case 's': String
case 'S': String With Len
case 'i': Integer
case 'r': Double
case 'u': Unitialized Object
case 'a': Array
case 'd': Dictionary
case 't': Struct
case 'e': Escript Error
case 'x': Unitialized Object

And of course, our switch default is an Escript error with the description (like the one you recived for the plain Hello World!)

Now, I will admit, yes, there SHOULD be some redundance checking here, as from what I can tell, there is, but not the right type. I will get with shini about a suitable rewrite (will require rewrite to String with length, array, struct, dict)

Posted: **Tue Oct 02, 2007 8:35 am**

test core for people to test this adjustment with. Is applied to Array, Struct, String with Length, and Dictionary to fix negative lengths being provided. Unable to test this atm as I am still rebuilding my main PC and have almost nothing reinstalled, hehe.

http://www.polserver.com/test-core.zip

Posted: **Wed Oct 03, 2007 12:10 am**

MuadDib wrote:Because you are trying to unpack BAD data, simple enough. Unless you can ensure the packed data is correct, do NOT use unpack on it.

I don't have much choice whether to use unpack or not when using AUX

MuadDib wrote:Now, you want to prevent against denial of service..... set up IP rules for the ports using firewall software/hardware to protect those ports. Easy enough.

This is a wrong approach - it's only masking the real issue, not eliminating it. Especially if it's exposed through a listening socket

MuadDib wrote:test core for people to test this adjustment with. Is applied to Array, Struct, String with Length, and Dictionary to fix negative lengths being provided.

Tested it and unfortunately it seems the fix is not really working.
S-1 -> Thread exception: basic_string
S-0 -> Unable to unpack string length. Length given was a negative!
S-10 -> Thread exception: bad allocation

Memory allocation exceptions are still caused by very large array sizes (this issue concerns me most, because it can bring down the whole server, not just one script).

I know the specs of packed format, just happens that I have some background in software security and like to "play" with things

Posted: **Wed Oct 03, 2007 3:31 am**

Miral wrote: S-1 -> Thread exception: basic_string
S-0 -> Unable to unpack string length. Length given was a negative!
S-10 -> Thread exception: bad allocation

Memory allocation exceptions are still caused by very large array sizes (this issue concerns me most, because it can bring down the whole server, not just one script).

Hrm so it catches the -0, but not the others...........

Means it's a STLPort issue i need to work around then. Allrighty, will get back on it

Posted: **Wed Oct 03, 2007 11:02 am**

by the way.... did you recompile also? Just curious

btw, uploaded a Debug core for you to try with the strings, same link as above.

Posted: **Wed Oct 03, 2007 3:25 pm**

Tried the new one (POL097-2007-08-23 RC5 Coregina (VS.NET 2003) compiled on Oct 2 2007 11:31:44), still no luck

Yeah, I've recompiled the script. Heading to bed now, 1:30 am here..

Posted: **Wed Oct 03, 2007 8:32 pm**

Ok.... may not be what you wanted, but rewrote it to function how it SHOULD.......

So far, strings with Length fixed (about to do the rest of them). Now it more accurately bitches at you if it is malformed, telling you HOW it is malformed. Should not truncate or pad, since that is signs of a malformed string set in this case.

Code: Select all

        Fixed : Exploit in unpacking Strings with length definition. Affected manual 
                building of packed versions such as in use by Aux connections etc.
                New error returns: 
                "Unable to unpack string length. Invalid length!" = 0 or less Length
                "Unable to unpack string length. Bad format. Colon not found!" = Duh.
                "Unable to unpack string length. String length excessive." = Said it 3,
                    but length was actually less.
                "Unable to unpack string length. String length short." = Said it was 3,
                    but length was actually more.