On Tue, Jul 17, 2012 at 04:48:47PM +0200, Marc-André Lureau wrote: > Hi > > On Tue, Jul 17, 2012 at 4:30 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote: > > Yeah I know there are many worrying places, for new code and new protocol > > additions, it would be nice to start thinking about this... > > I'm not seeing this as a blocking issue, but this is getting more and more > > scary nonetheless... > > I don't think you have reasons to be worried here. What will a browser > do if it receives a message or say an image with a gigantic size? it > will probably keep reading an decoding it, until you run out of RAM, > no? That's the same for Spice. Checking server sizes doesn't make > sense. Checking out-of-bounds of memory / array lead by a decoding > logic (no matter how deep in the code) is what we should be careful > about (think about dictionnaries, or cache etc). Ok, so monitors_max is a guint, the second argument to g_ptr_array_set_size is a gint, and the GPtrArray code doesn't seem to handle. A quick reading of GPtrArray code didn't make me feel confident that it would do the right thing when size > G_INT_MAX / sizeof(void *), which could cause a buffer overflow when very huge values are used (I haven't carefully checked that). > There is nothing wrong in OOM if the server tells us we have 1024 > maximum monitors on the guest for example, Is this max value coming from the server, or is it the guest qxl driver telling the server about how many monitors it supports? Christophe
Attachment:
pgpbmu8HTlbfJ.pgp
Description: PGP signature
