[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: Possible DoS in whois (1.31), caused by memory exhaustion
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2019-12-03 13:53:24
Message-ID: CAK1hOcOg6U5UVV0DT=Ca5W7EbeXj+MLcJ33OW_BE3Ae8MPgYFg () mail ! gmail ! com
[Download RAW message or body]

I limited the size to 32k now.

On Mon, Dec 2, 2019 at 10:37 PM Erez Turjeman <erezto@gmail.com> wrote:
> 
> 
> 
> On Mon, Dec 2, 2019 at 4:19 PM Tito <farmatito@tiscali.it> wrote:
> > 
> > On 12/2/19 9:50 PM, Erez Turjeman wrote:
> > > I don't think that the RFC itself defines the any constraints on response \
> > > length or maximum count of iterations between client and server. However, the \
> > > implementation provided in Debian \
> > > (https://git.launchpad.net/ubuntu/+source/whois/tree/whois.c#n780), assumes the \
> > > each response from server should be treated as a complete entry (i.e. not \
> > > fragmented) and thus avoids using realloc. Regards,
> > > Erez
> > > 
> > > On Mon, Dec 2, 2019 at 2:59 PM Tito <farmatito@tiscali.it \
> > > <mailto:farmatito@tiscali.it>> wrote: 
> > > On 12/2/19 4:54 PM, Erez Turjeman wrote:
> > > > The implementation of `whois` in busybox 1.31 calls xrealloc without limiting \
> > > > the size argument, which can lead to a memory exhaustion, \
> > > > https://git.busybox.net/busybox/tree/networking/whois.c?h=1_31_stable#n63. A \
> > > > rogue server can simply reply with an endless response until crashing the \
> > > > client. Regards,
> > > > Erez
> > > > --
> > > > Erez Turjeman
> > > > erezto@gmail.com <mailto:erezto@gmail.com> <mailto:erezto@gmail.com \
> > > > <mailto:erezto@gmail.com>> 
> > > 
> > > Hi,
> > > shouldn't the app simply exit if memory is exhausted and thus free the memory \
> > > again? 
> > > Is there any maximum size for the server reply or do you think an arbitrary
> > > maximum value should be hardcoded?
> > > 
> > > // Die if we can't resize previously allocated memory.  (This returns a pointer
> > > // to the new memory, which may or may not be the same as the old memory.
> > > // It'll copy the contents to a new chunk and free the old one if necessary.)
> > > void* FAST_FUNC xrealloc(void *ptr, size_t size)
> > > {
> > > ptr = realloc(ptr, size);
> > > if (ptr == NULL && size != 0)
> > > bb_die_memory_exhausted();
> > > return ptr;
> > > }
> > > 
> > > Ciao,
> > > Tito
> > > 
> > > 
> > > 
> > 
> > Hi,
> > seems to me they use a hardcoded max size of 2000.
> > 
> > char *do_query(const int sock, const char *query)
> > {
> > char *temp, *p, buf[2000];
> > 
> > 
> > 
> > Ciao,
> > Tito
> > 
> > P.S: please don't top post because it makes threads hard to read.
> 
> 
> Yes, it seems that the length of that local buffer is assumed to be long enough to \
> hold valid responses. Perhaps aborting and printing an error message in case the \
> buffer was filled with no CR/LF at the end will help the user diagnose possible \
> issues. This of course on top of avoiding using realloc with no constraints. 
> 
> _______________________________________________
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread]