[Opensim-dev] Packet text field overflows

[Justin Clark-Casey]

Jeff Ames wrote:
> Hello,
>
> In libSL, there are a number of packets with string fields, with a
> maximum length of either 255 or 1500 characters.  If OpenSim sends a
> string longer than this, then libSL will throw an exception.
>
> One fix to this is to replace the existing bits of ClientView.cs,
> generally something like this:
>
>     myPacket.text = Helpers.StringToField(text)
>
> with something like this:
>
>     myPacket.text = Helpers.StringToField(text.Substring(0,
> Math.Min(1499, text.Length)))
>
> (The 1499 is because StringToField will add a null terminator.)
>
> This approach, however, requires OpenSim to hard-code some of the
> internal details of these packets.  Another solution, which would
> require libSL changes, would be to define something like a
> libSL.BoundedString class with a MaxLength member OpenSim could query
> and use to apply Substring() appropriately.
>
> However, these methods only solve the problem of outgoing data.
> Currently it seems we're trusting the client not to send overly long
> strings for fields that should be bounded.  This issue came up this
> time because OpenSim was incorrectly converting a non-ASCII string
> into a string of hex values, which quickly expanded past the max field
> length.  If a rogue client sent overly long strings, we could
> conceivably have two crashes -- a MalformedDataException when libSL
> tries to construct the incoming packet, and an OverflowException when
> OpenSim sends that information back out.
>
> One possible solution would be to call Substring in the property set()
> methods in libSL instead of throwing exceptions, and maybe output some
> warning message.  Or we could wrap all the libSL interactions with
> try/catch blocks and log failed packet creation.
>
> Thoughts?
>
>   
I think we certainly need to protect ourselves against problems of this 
kind.  As for the location, it seems to me that doing it in libSL is 
nicer, since presumably these issues apply to all libSL users rather 
than just us?  Also, it feels like a protocol issue rather than an 
OpenSim issue. 

On the con side, I imagine getting such a patch into libSL is much 
harder than just doing it ourselves (remembering the whole 
LLUUID.ToString() business), and doing our own patch on top of libSL is 
also harder to maintain. 

On balance, though, I think patching libSL is still better (perhaps I'm 
just saying that because I'm not the one doing it :-)

Perhaps DrSchofld has some comments, since I know he's doing work with 
libsecondlife?

Regards,

-- 
justincc
Justin Clark-Casey