search

ArcticaProject / nx-libs

nx-libs
Other
120 stars 39 forks source link

CVE backports incomplete or wrong #29

Closed sunweaver closed 9 years ago

sunweaver commented 9 years ago

The below has been filed via X2Go BTS [1] by Ulrich Sibiller (@uli42)and should be fixed here (3.6.x branch) first before rebasing against 3.5.0.x.

""" Recently a lot of CVE fixes have been added to nx-libs.

E.g. debian/patches/1027-render-check-request-size-before-reading-it-CVE.full.patch and debian/patches/1028-render-unvalidated-lengths-in-Render-extn.-swap.full.patch add missing checks to nx-X11/programs/Xserver/render/render.c.

However, there's a file called nx-X11/programs/Xserver/hw/nxagent/NXrender.c which is derived from render.c and in that file those checks are missing, too.

(I suspect the original render/render.c is not used at all in favour of hw/nxagent/NXrender.c but I am not 100% sure here.)

If render.c is used a all (I am not sure) the patches should be extended to also fix NXrender.c. If render.c is not used it should be removed and the patches should be applied to NXrender.c instead.

There might be more cases like this, I only picked this one as an example. """

[1] https://bugs.x2go.org/879

mikedep333 commented 9 years ago

I've started looking into this.

mikedep333 commented 9 years ago

Update: It looks like the NX* files do depend on some of the files under nx-X11/programs/Xserver/render/, such as animcur.c.

mikedep333 commented 9 years ago

I created branch pr/render-cleanup-and-cve-fixes for the cleanup and CVE fixes in the Render (XRender) code specifically. I have not submitted a PR yet.

mikedep333 commented 9 years ago

I have identified what files under nx-X11/programs/Xserver/hw/nxagent/ are duplicates/derivatives of other files. I am keeping track of it in this spreadsheet. I will probably work on the nx-X11/programs/Xserver/dix/ files next.

mikedep333 commented 9 years ago

I believe that I have submitted all the necessary PRs for CVE fixes (#36, #45 & #46). I added a new column to the above-mentioned spreadsheet called "CVE Fix PR".

sunweaver commented 9 years ago

All PRs have been merged.

uli42 commented 9 years ago

The spreadsheet contains some files with the comment "appears to be unique". While this is true looking at the nx tree only it is not true if you look at where nxagent orginates from. nxagent is an adapted xnest so these files can be found in hw/nxnest in the current Xorg tree. If there are any CVEs regarding xnest these should be backported to nxagent.

So I think this should be reopened.

mikedep333 commented 9 years ago

@uli42 I identified a total of 39 files in that spreadsheet as having been derived from xnest. They have the "Original" file listed as a file under "hw/xnest/". I believe that every file that "appears to be unique" was written by NoMachine. If I made a mistake, please point out the specific file(s).

Also, I reviewed every X.org security advisory and I did not see any vulnerability specific to xnest.

uli42 commented 9 years ago

Sorry, my fault. I have not seen that xnest references are already contained in the list. Should have looked closer...

On Sun, Jun 14, 2015 at 1:03 AM, Mike DePaulo notifications@github.com wrote:

@uli42 https://github.com/uli42 I identified a total of 39 files in that spreadsheet as having been derived from xnest. They have the "Original" file listed as a file under "hw/xnest/". I believe that every file that "appears to be unique" was written by NoMachine. If I made a mistake, please point out the specific file(s).

Also, I reviewed every X.org security advisory http://www.x.org/wiki/Development/Security/ and I did not see any vulnerability specific to xnest.

— Reply to this email directly or view it on GitHub https://github.com/ArcticaProject/nx-libs/issues/29#issuecomment-111759023 .

mikedep333 commented 9 years ago

@uli42 No problem :)