Bug 12572 (CVE-2017-9461) - fd_open_atomic() can loop indefinitely when trying to open an invalid symlink with O_CREAT
Summary: fd_open_atomic() can loop indefinitely when trying to open an invalid symlink...
Status: RESOLVED FIXED
Alias: CVE-2017-9461
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.2.14
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-09 19:44 UTC by YOUZHONG YANG
Modified: 2017-07-04 19:49 UTC (History)
3 users (show)

See Also:


Attachments
git-am fix for master (4.26 KB, patch)
2017-02-13 22:25 UTC, Jeremy Allison
no flags Details
Correct patch for master (8.44 KB, patch)
2017-02-15 01:13 UTC, Jeremy Allison
no flags Details
Better comment and commit message. (8.66 KB, patch)
2017-02-15 01:44 UTC, Jeremy Allison
no flags Details
Patch submitted to samba-technical (8.39 KB, patch)
2017-02-15 17:44 UTC, Jeremy Allison
no flags Details
Pair programmed with Ralph. (9.91 KB, patch)
2017-02-16 00:19 UTC, Jeremy Allison
slow: review+
Details
git-am fix for 4.6.next, 4.5.next. (10.55 KB, patch)
2017-02-16 21:32 UTC, Jeremy Allison
slow: review+
Details
git-am fix for 4.4.next (5.03 KB, patch)
2017-02-16 21:33 UTC, Jeremy Allison
slow: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description YOUZHONG YANG 2017-02-09 19:44:40 UTC
From: Youzhong Yang 
Sent: Thursday, February 09, 2017 9:55 AM
To: samba-technical@lists.samba.org
Subject: smbd enters infinite loop when trying to open an invalid symlink with O_CREAT

Reproduction is simple:

-	On the server, create a symbolic link pointing to a nonexistent file
-	Use the following program on Windows to open the symlink:

#include <stdio.h>
#include <tchar.h>
#include <windows.h>

int _tmain(int argc, _TCHAR* argv[])
{
       HANDLE h;
       
       h = CreateFile(argv[1], 
              GENERIC_READ | GENERIC_WRITE, 
              FILE_SHARE_READ|FILE_SHARE_WRITE,
              NULL, CREATE_ALWAYS, 0, NULL);
       printf("hit <cr>\n");
       getchar();
       CloseHandle(h);

       return 0;
}

I believe fd_open_atomic() needs to handle the symlink case specifically. Please advise.

Thanks,

--Youzhong
Comment 1 Jeremy Allison 2017-02-13 22:06:45 UTC
Found and fixed the bug. Patch to follow. We'll also need a regression test case for this (also to follow).

Jeremy.
Comment 2 Jeremy Allison 2017-02-13 22:07:17 UTC
Marking private until we decide if this is a security issue or not.
Comment 3 Jeremy Allison 2017-02-13 22:25:51 UTC
Created attachment 12925 [details]
git-am fix for master

Not complete - needs regression test case but I wanted to keep a record of the fix here.
Comment 4 Jeremy Allison 2017-02-15 01:13:52 UTC
Created attachment 12928 [details]
Correct patch for master

Includes regression test.
Comment 5 Jeremy Allison 2017-02-15 01:14:17 UTC
Opening back up, not a security bug I don't think.
Comment 6 Jeremy Allison 2017-02-15 01:44:41 UTC
Created attachment 12929 [details]
Better comment and commit message.
Comment 7 Jeremy Allison 2017-02-15 17:44:04 UTC
Created attachment 12942 [details]
Patch submitted to samba-technical
Comment 8 Jeremy Allison 2017-02-16 00:19:20 UTC
Created attachment 12946 [details]
Pair programmed with Ralph.
Comment 9 Ralph Böhme 2017-02-16 09:10:01 UTC
Comment on attachment 12946 [details]
Pair programmed with Ralph.

Perfect!
Comment 10 Jeremy Allison 2017-02-16 21:32:39 UTC
Created attachment 12948 [details]
git-am fix for 4.6.next, 4.5.next.

Cherry-pick from master.
Comment 11 Jeremy Allison 2017-02-16 21:33:55 UTC
Created attachment 12949 [details]
git-am fix for 4.4.next

Cherry-pick from master. Doesn't include regression test (doesn't apply cleanly, not worth back-porting from master to 4.4.next only).
Comment 12 Ralph Böhme 2017-02-17 04:10:01 UTC
Reassigning to Karolin for inclusion in 4.4, 4.5 and 4.6.
Comment 13 Karolin Seeger 2017-02-17 11:01:33 UTC
(In reply to Ralph Böhme from comment #12)
Pushed to autobuild-v4-{6,5,4}-test.
Comment 14 Karolin Seeger 2017-02-21 08:49:27 UTC
(In reply to Karolin Seeger from comment #13)
Pushed to all branches.
Closing out bug report.

Thanks!
Comment 15 Salvatore Bonaccorso 2017-06-07 04:24:04 UTC
Hi

The issue got CVE-2017-9461 assigned. Although probably minor, is there a reason it was not considered as security bug per se? According to downstream report at https://bugs.debian.org/864291 it could be used for a denial of service (high cpu usage, memory exhaustion).
Comment 16 Jeremy Allison 2017-06-07 15:57:46 UTC
(In reply to Salvatore Bonaccorso from comment #15)

Just seemed too minor to log as a security fix. Just IMHO.