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
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

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.