- Dec 20, 2014
-
-
Julien Cristau authored
Signed-off-by: Julien Cristau <jcristau@debian.org>
-
- Dec 09, 2014
-
-
Julien Cristau authored
Signed-off-by: Julien Cristau <jcristau@debian.org>
-
GetHosts saves the pointer to allocated memory in *data, and then wants to bounds-check writes to that region, but was mistakenly using a bare 'data' instead of '*data'. Also, data is declared as void **, so we need a cast to turn it into a byte pointer so we can actually do pointer comparisons. Signed-off-by: Keith Packard <keithp@keithp.com> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 1559a943) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
The 'n' parameter must be surrounded by parens in both places to prevent precedence from mis-computing things. Signed-off-by: Keith Packard <keithp@keithp.com> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 9802a016) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
We're using compiler compatibility settings which generate warnings when a variable is declared after the first statement. Signed-off-by: Keith Packard <keithp@keithp.com> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 61b17c0f) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
When the local types used to walk the DBE request were changed, this changed the type of the parameter passed to the DDX SwapBuffers API, but there wasn't a matching change in the API definition. At this point, with the API frozen, I just stuck a new variable in with the correct type. Because we've already bounds-checked nStuff to be smaller than UINT32_MAX / sizeof(DbeSwapInfoRec), we know it will fit in a signed int without overflow. Signed-off-by: Keith Packard <keithp@keithp.com> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit b20912c3) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
On a system where sizeof(unsigned) != sizeof(intptr_t), the unary bitwise not operation will result in a mask that clears all high bits from temp_buf in the expression: temp_buf = (temp_buf + mask) & ~mask; Signed-off-by: Robert Morell <rmorell@nvidia.com> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 7e7630bb) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
v2: Handle more multiplies in indirect_reqsize.c (Julien Cristau) Reviewed-by: Julien Cristau <jcristau@debian.org> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit e883c170) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
v2: Fix single versus vendor-private length checking for ARB_imaging subset extensions. (Julien Cristau) v3: Fix single versus vendor-private length checking for ARB_imaging subset extensions. (Julien Cristau) Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Julien Cristau <jcristau@debian.org> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 984583a4) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Reviewed-by: Keith Packard <keithp@keithp.com> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 44ba149f) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Reviewed-by: Keith Packard <keithp@keithp.com> Reviewed-by: Julien Cristau <jcristau@debian.org> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit afe17702) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Reviewed-by: Keith Packard <keithp@keithp.com> Reviewed-by: Julien Cristau <jcristau@debian.org> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit c91e4abc) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
This is a half-measure until we start passing request length into the varsize function, but it's better than the nothing we had before. v2: Verify that there's at least a large render header's worth of dataBytes (Julien Cristau) Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit a33a939e) Signed-off-by: Julien Cristau <jcristau@debian.org> Conflicts: glx/glxcmds.c
-
v2: Fix constants in __glXMap2fReqSize (Michal Srb) Validate w/h/d for proxy targets too (Keith Packard) v3: Fix Map[12]Size to correctly reject order == 0 (Julien Cristau) Reviewed-by: Keith Packard <keithp@keithp.com> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 698888e6) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Julien Cristau authored
v2: Remove can't-happen comparison for cmdlen < 0 (Michal Srb) Reviewed-by: Adam Jackson <ajax@redhat.com> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Julien Cristau <jcristau@debian.org> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit be09e0c9) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
These are paranoid about integer overflow, and will return -1 if their operation would overflow a (signed) integer or if either argument is negative. Note that RenderLarge requests are sized with a uint32_t so in principle this could be sketchy there, but dix limits bigreqs to 128M so you shouldn't ever notice, and honestly if you're sending more than 2G of rendering commands you're already doing something very wrong. v2: Use INT_MAX for consistency with the rest of the server (jcristau) v3: Reject negative arguments (anholt) Reviewed-by: Keith Packard <keithp@keithp.com> Reviewed-by: Julien Cristau <jcristau@debian.org> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 2a5cbc17) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Without this we'd reject the request with BadLength. Note that some old versions of Mesa had a bug in the same place, and would _send_ zero bytes of image data; these will now be rejected, correctly. Reviewed-by: Keith Packard <keithp@keithp.com> Reviewed-by: Julien Cristau <jcristau@debian.org> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 13d36923) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
If the computed reply size is negative, something went wrong, treat it as an error. v2: Be more careful about size_t being unsigned (Matthieu Herrb) v3: SIZE_MAX not SIZE_T_MAX (Alan Coopersmith) Reviewed-by: Julien Cristau <jcristau@debian.org> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 717a1b37) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Before this we'd just clamp the image size to 0, which was just hideously stupid; if the parameters were such that they'd overflow an integer, you'd allocate a small buffer, then pass huge values into (say) ReadPixels, and now you're scribbling over arbitrary server memory. Reviewed-by: Keith Packard <keithp@keithp.com> Reviewed-by: Julien Cristau <jcristau@debian.org> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit ab2ba933) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
If the size computation routine returns -1 we should just reject the request outright. Clamping it to zero could give an attacker the opportunity to also mangle cmdlen in such a way that the subsequent length check passes, and the request would get executed, thus passing data we wanted to reject to the renderer. Reviewed-by: Keith Packard <keithp@keithp.com> Reviewed-by: Julien Cristau <jcristau@debian.org> Reviewed-by: Michal Srb <msrb@suse.com> Reviewed-by: Andy Ritger <aritger@nvidia.com> Signed-off-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit 23fe7718) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit f4afd53f) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 2df83bb1) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit d153a85f) Signed-off-by: Julien Cristau <jcristau@debian.org> Conflicts: test/Makefile.am
-
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit a0ece23a) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 5d3a788a) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Julien Cristau authored
Otherwise we may be reading outside of the client request. Signed-off-by: Julien Cristau <jcristau@debian.org> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> (cherry picked from commit b5f9ef03) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 3df2fcf1) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> Reviewed-by: Julien Cristau <jcristau@debian.org> (cherry picked from commit d155b7a8) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 0a6085aa) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 32a95fb7) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 7553082b) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Multiple functions in the Xinput extension handling of requests from clients failed to check that the length of the request sent by the client was large enough to perform all the required operations and thus could read or write to memory outside the bounds of the request buffer. This commit includes the creation of a new REQUEST_AT_LEAST_EXTRA_SIZE macro in include/dix.h for the common case of needing to ensure a request is large enough to include both the request itself and a minimum amount of extra data following the request header. Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 73c63afb) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
ProcDbeSwapBuffers() has a 32bit (n) length value that it uses to read from a buffer. The length is never validated, which can lead to out of bound reads, and possibly returning the data read from out of bounds to the misbehaving client via an X Error packet. SProcDbeSwapBuffers() swaps data (for correct endianness) before handing it off to the real proc. While doing the swapping, the length field is not validated, which can cause memory corruption. v2: reorder checks to avoid compilers optimizing out checks for overflow that happen after we'd already have done the overflowing multiplications. Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 2ef42519) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
ProcDRI2GetBuffers() tries to validate a length field (count). There is an integer overflow in the validation. This can cause out of bound reads and memory corruption later on. Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> Reviewed-by: Julien Cristau <jcristau@debian.org> (cherry picked from commit 6692670f) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
Force use of 64-bit integers when evaluating data provided by clients in 32-bit fields which can overflow when added or multiplied during checks. Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit e0e11644) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
RegionSizeof contains several integer overflows if a large length value is passed in. Once we fix it to return 0 on overflow, we also have to fix the callers to handle this error condition v2: Fixed limit calculation in RegionSizeof as pointed out by jcristau. Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> Reviewed-by: Julien Cristau <jcristau@debian.org> (cherry picked from commit 97015a07) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
GetHosts() iterates over all the hosts it has in memory, and copies them to a buffer. The buffer length is calculated by iterating over all the hosts and adding up all of their combined length. There is a potential integer overflow, if there are lots and lots of hosts (with a combined length of > ~4 gig). This should be possible by repeatedly calling ProcChangeHosts() on 64bit machines with enough memory. This patch caps the list at 1mb, because multi-megabyte hostname lists for X access control are insane. Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit bc8e2043) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
ProcPutImage() calculates a length field from a width, left pad and depth specified by the client (if the specified format is XYPixmap). The calculations for the total amount of memory the server needs for the pixmap can overflow a 32-bit number, causing out-of-bounds memory writes on 32-bit systems (since the length is stored in a long int variable). Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit eeae42d6) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
authdes_ezdecode() calls malloc() using a length provided by the connection handshake sent by a newly connected client in order to authenticate to the server, so should be treated as untrusted. It didn't check if malloc() failed before writing to the newly allocated buffer, so could lead to a server crash if the server fails to allocate memory (up to UINT16_MAX bytes, since the len field is a CARD16 in the X protocol). Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> (cherry picked from commit 90cc925c) Signed-off-by: Julien Cristau <jcristau@debian.org>
-
This function can return NULL; make sure every caller tests for that. Reviewed-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Keith Packard <keithp@keithp.com> (cherry picked from commit 61a292ad) Signed-off-by: Julien Cristau <jcristau@debian.org>
-