Skip to content

arch cleanup (CRAY/WORD64) + X.Org CVE-2013-7439 #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Apr 16, 2015
Merged

arch cleanup (CRAY/WORD64) + X.Org CVE-2013-7439 #12

merged 2 commits into from
Apr 16, 2015

Conversation

sunweaver
Copy link
Member

  1. Drop CRAY support from NX.
  2. Fix X.Org CVE-2013-7439

@sunweaver
Copy link
Member Author

Hi Mike#2,

is it possible that you missed CVE-2013-7439 on your CVE hunt? Stefan Baur pointed me towards it after he had read this Heise post [1].

As this fix got committed to X.Org after removal of WORD64 architecture types (i.e., CRAY), but WORD64 builds were also affected (but without a patch from X.Org), I simply decided dropping CRAY support from NX (like in X.Org back in 2013 [1]) before fixing this CVE issue.

Can you please review? Thanks.

[1] http://www.heise.de/newsticker/meldung/Alte-Xorg-Luecke-bedroht-haufenweise-Drittsoftware-2606536.html
[2] http://lists.x.org/archives/xorg-devel/2013-September/037716.html

@mikedep333
Copy link
Contributor

Looks like I missed CVE-2013-7439 because:

  1. It was not listed in the X.org security page until 2 days ago.
  2. RHEL5's libX11 was never patched for it. Specifically, the latest version of libX11 in RHEL 5 is 1.0.3-11.el5_7.1, which was released on 2011-09-30

@mikedep333
Copy link
Contributor

Actually, the vulnerability was fixed in 2013, but discovered in 2015. So a CVE-2013 was not assigned until 2015.

@mikedep333
Copy link
Contributor

Other than that, the fix for X.Org CVE-2013-7439 looks good to me. Now I will review the other commit.

@sunweaver
Copy link
Member Author

Looks like I missed CVE-2013-7439 because:

  1. It was not listed in the X.org security page until 2 days ago.
  2. RHEL5's libX11 was never patched for it. Specifically, the latest version of libX11 in RHEL 5 is
    1.0.3-11.el5_7.1, which was released on 2011-09-30

Nice. Ok.

@sunweaver
Copy link
Member Author

Actually, the vulnerability was fixed in 2013, but discovered in 2015. So a CVE-2013 was not
assigned until 2015.

This really makes me grin... ;-)

sunweaver and others added 2 commits April 16, 2015 14:09
@sunweaver
old-arch cleanup: Drop CRAY support (i.e., drop code related to CRAY,…
bad6779 
… _CRAY, WORD64, WORD64ALIGN, MUSTCOPY, UNSIGNEDBITFIELDS definitions).
@sunweaver
MakeBigReq: don't move the last word, already handled by Data32 (X.Org …
ac9fbaa 
…CVE-2013-7439).

 MakeBigReq inserts a length field after the first 4 bytes of the request
 (after req->length), pushing everything else back by 4 bytes.

 The current memmove moves everything but the first 4 bytes back. If a
 request aligns to the end of the buffer pointer when MakeBigReq is
 invoked for that request, this runs over the buffer. Instead, we need to
 memmove minus the first 4 bytes (which aren't moved), minus the last 4
 bytes (so we still align to the previous tail).

 The 4 bytes that fell out are already handled with Data32, which will
 handle the buffermax correctly.

 The case where req->length = 1 was already not functional.

 Reported by Abhishek Arya <inferno@chromium.org> (against X.Org BTS).

 https://bugzilla.mozilla.org/show_bug.cgi?id=803762

 Reviewed-by: Jeff Muizelaar <jmuizelaar@mozilla.com>
 Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
 Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
 Rebased-for-NX: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>
@sunweaver sunweaver force-pushed the pr/arch-cleanup.CRAY branch from 10c40f0 to ac9fbaa Compare April 16, 2015 12:16
@mikedep333
Copy link
Contributor

I have finished reviewing the CRAY commit.

(I made some comments and Sunweaver and I have been discussing them via comments and IRC.)

@mikedep333
Copy link
Contributor

Hmm, I accidentally commented on the individual commits rather than the commits as part of the PR. Oh well, @sunweaver still saw the comments..

@mikedep333
Copy link
Contributor

Looks good with the updated CRAY commit.

mikedep333 added a commit that referenced this pull request Apr 16, 2015
@mikedep333
Merge pull request #12 from sunweaver/pr/arch-cleanup.CRAY
8b15775 
arch cleanup (CRAY/WORD64) + X.Org CVE-2013-7439
@mikedep333 mikedep333 merged commit 8b15775 into ArcticaProject:3.6.x Apr 16, 2015
@sunweaver sunweaver deleted the pr/arch-cleanup.CRAY branch April 21, 2015 18:34
@sunweaver sunweaver added this to the 3.6.0.x milestone May 16, 2015
@uli42 uli42 mentioned this pull request May 4, 2016
Closed
@pgotch pgotch mentioned this pull request Mar 13, 2018
Closed
@JohnCaiJun JohnCaiJun mentioned this pull request Nov 28, 2018
Open
@uli42 uli42 mentioned this pull request May 15, 2019
Open
@uli42 uli42 mentioned this pull request Oct 9, 2019
Open
uli42 added a commit to uli42/nx-libs that referenced this pull request Oct 2, 2020
@uli42
nxagent: fix stack smashing
aa5ef6c 
In compext Atom has the size of XlibAtom. Therefore calling functions
of Compext.c requires to use/pass XlibAtom. Same for Window/XlibWindow.

==15438==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffffffcdc0 at pc 0x5555556a81b5 bp 0x7fffffffcd10 sp 0x7fffffffcd08
WRITE of size 8 at 0x7fffffffcdc0 thread T0
    #0 0x5555556a81b4 in NXGetCollectedProperty nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c:4124
    #1 0x5555557d0488 in nxagentCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Clipboard.c:1202
    #2 0x555555723340 in nxagentHandleCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:3923
    #3 0x55555571d4db in nxagentHandleProxyEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:3007
    ArcticaProject#4 0x55555571bb92 in nxagentHandleClientMessageEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:2595
    ArcticaProject#5 0x555555717dfc in nxagentDispatchEvents nx-X11/programs/Xserver/hw/nxagent/Events.c:1827
    ArcticaProject#6 0x555555750813 in nxagentBlockHandler nx-X11/programs/Xserver/hw/nxagent/Handlers.c:437
    ArcticaProject#7 0x5555556c1b5d in BlockHandler nx-X11/programs/Xserver/dix/dixutils.c:403
    ArcticaProject#8 0x5555556d47ff in WaitForSomething nx-X11/programs/Xserver/os/WaitFor.c:232
    ArcticaProject#9 0x555555665b22 in Dispatch nx-X11/programs/Xserver/hw/nxagent/NXdispatch.c:365
    ArcticaProject#10 0x5555555ed760 in main nx-X11/programs/Xserver/dix/main.c:350
    ArcticaProject#11 0x7ffff604909a in __libc_start_main ../csu/libc-start.c:308
    ArcticaProject#12 0x5555555edc09 in _start (nx-X11/programs/Xserver/nxagent+0x99c09)

Address 0x7fffffffcdc0 is located in stack of thread T0 at offset 32 in frame
    #0 0x5555557d0324 in nxagentCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Clipboard.c:1190

  This frame has 5 object(s):
    [32, 36) 'atomReturnType' <== Memory access at offset 32 partially overflows this variable
    [96, 100) 'resultFormat'
    [160, 168) 'ulReturnItems'
    [224, 232) 'ulReturnBytesLeft'
    [288, 296) 'pszReturnData'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c:4124 in NXGetCollectedProperty
...
@uli42 uli42 mentioned this pull request Oct 7, 2020
Merged
sunweaver pushed a commit to uli42/nx-libs that referenced this pull request Oct 17, 2020
@uli42 @sunweaver
nxagent: fix stack smashing
ae03770 
In compext Atom has the size of XlibAtom. Therefore calling functions
of Compext.c requires to use/pass XlibAtom. Same for Window/XlibWindow.

==15438==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffffffcdc0 at pc 0x5555556a81b5 bp 0x7fffffffcd10 sp 0x7fffffffcd08
WRITE of size 8 at 0x7fffffffcdc0 thread T0
    #0 0x5555556a81b4 in NXGetCollectedProperty nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c:4124
    #1 0x5555557d0488 in nxagentCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Clipboard.c:1202
    #2 0x555555723340 in nxagentHandleCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:3923
    #3 0x55555571d4db in nxagentHandleProxyEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:3007
    ArcticaProject#4 0x55555571bb92 in nxagentHandleClientMessageEvent nx-X11/programs/Xserver/hw/nxagent/Events.c:2595
    ArcticaProject#5 0x555555717dfc in nxagentDispatchEvents nx-X11/programs/Xserver/hw/nxagent/Events.c:1827
    ArcticaProject#6 0x555555750813 in nxagentBlockHandler nx-X11/programs/Xserver/hw/nxagent/Handlers.c:437
    ArcticaProject#7 0x5555556c1b5d in BlockHandler nx-X11/programs/Xserver/dix/dixutils.c:403
    ArcticaProject#8 0x5555556d47ff in WaitForSomething nx-X11/programs/Xserver/os/WaitFor.c:232
    ArcticaProject#9 0x555555665b22 in Dispatch nx-X11/programs/Xserver/hw/nxagent/NXdispatch.c:365
    ArcticaProject#10 0x5555555ed760 in main nx-X11/programs/Xserver/dix/main.c:350
    ArcticaProject#11 0x7ffff604909a in __libc_start_main ../csu/libc-start.c:308
    ArcticaProject#12 0x5555555edc09 in _start (nx-X11/programs/Xserver/nxagent+0x99c09)

Address 0x7fffffffcdc0 is located in stack of thread T0 at offset 32 in frame
    #0 0x5555557d0324 in nxagentCollectPropertyEvent nx-X11/programs/Xserver/hw/nxagent/Clipboard.c:1190

  This frame has 5 object(s):
    [32, 36) 'atomReturnType' <== Memory access at offset 32 partially overflows this variable
    [96, 100) 'resultFormat'
    [160, 168) 'ulReturnItems'
    [224, 232) 'ulReturnBytesLeft'
    [288, 296) 'pszReturnData'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow nx-X11/programs/Xserver/hw/nxagent/compext/Compext.c:4124 in NXGetCollectedProperty
...
@uli42 uli42 mentioned this pull request Mar 2, 2021
Open
@uli42 uli42 mentioned this pull request Mar 16, 2021
Closed
@uli42 uli42 mentioned this pull request Nov 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mikedep333 mikedep333

Labels
code cleanup security
Projects
None yet
Milestone
3.6.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@sunweaver @mikedep333