security fix to wxSingleInstanceChecker: check if the lock file was really created...

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@27542 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/docs/latex/wx/snglinst.tex b/docs/latex/wx/snglinst.tex
index bc35b396740a4d2b1f0638cc8b1c62f86676a9d7..54517eaea3afa1d8a16266e8437b5c3279b50346 100644 (file)
--- a/docs/latex/wx/snglinst.tex
+++ b/docs/latex/wx/snglinst.tex
@@ -104,6 +104,15 @@ instance is running - use
 \helpref{IsAnotherRunning()}{wxsingleinstancecheckerisanotherrunning} to check
 for it.
 
+\wxheading{Note}
+
+One of possible reasons while Create may fail on Unix is that the lock file
+used for checking already exists but was not created by the user.
+Therefore applications shouldn't treat failure of this function as fatal
+condition, because doing so would open them to the possibility of a Denial of
+Service attack. Instead, they should alert the user about the problem and
+offer to continue execution without checking if another instance is running.
+
 \membersection{wxSingleInstanceChecker::IsAnotherRunning}\label{wxsingleinstancecheckerisanotherrunning}
 
 \constfunc{bool}{IsAnotherRunning}{\void}

diff --git a/src/unix/snglinst.cpp b/src/unix/snglinst.cpp
index df4971de418091356f281f4087d7f64b5eff37e5..f3c5b3e6819321845e8cf0cafd58f79ac29d4ebc 100644 (file)
--- a/src/unix/snglinst.cpp
+++ b/src/unix/snglinst.cpp
@@ -180,6 +180,17 @@ LockResult wxSingleInstanceCheckerImpl::CreateLockFile()
 
             fsync(m_fdLock);
 
+            // change file's permission so that only this user can access it:
+            if ( chmod(m_nameLock.fn_str(), S_IRUSR | S_IWUSR) != 0 )
+            {
+                wxLogSysError(_("Failed to set permissions on lock file '%s'"),
+                              m_nameLock.c_str());
+
+                Unlock();
+                
+                return LOCK_ERROR;
+            }
+
             return LOCK_CREATED;
         }
         else // failure: see what exactly happened
@@ -226,6 +237,26 @@ bool wxSingleInstanceCheckerImpl::Create(const wxString& name)
             return FALSE;
     }
 
+    // Check if the file is owned by current user and has 0600 permissions.
+    // If it doesn't, it's a fake file, possibly meant as a DoS attack, and
+    // so we refuse to touch it:
+    wxStructStat stat;
+    if ( wxStat(name, &stat) != 0 )
+    {
+        wxLogSysError(_("Failed to inspect the lock file '%s'"), name.c_str());
+        return false;
+    }
+    if ( stat.st_uid != getuid() )
+    {
+        wxLogError(_("Lock file '%s' has incorrect owner."), name.c_str());
+        return false;
+    }
+    if ( stat.st_mode != (S_IFREG | S_IRUSR | S_IWUSR) )
+    {
+        wxLogError(_("Lock file '%s' has incorrect permissions."), name.c_str());
+        return false;
+    }
+
     // try to open the file for reading and get the PID of the process
     // which has it
     wxFile file(name, wxFile::read);