[Pcsclite-git-commit] [PCSC] 03/06: SCardCancel(): fix a possible use after free

[Ludovic Rousseau]

This is an automated email from the git hooks/post-receive script.

rousseau pushed a commit to branch master
in repository PCSC.

commit 9a21c82330fc4b73ac00faf2c5cf948da5f8ab07
Author: Ludovic Rousseau <ludovic.rousseau at free.fr>
Date:   Fri Jan 27 10:26:11 2017 +0100

    SCardCancel(): fix a possible use after free
    
    From Maksim Ivanov:
    " It seems that there are still some issues with regard to the blocking
    requests cancellation functionality in PC/SC-Lite:
    
    2. Use-after-free possible in SCardCancel.
    
       This is probably a low-severity issue, as the deallocated memory will
       be accessed only for reading an int, which would be then used for
       deciding whether to fail fast or to send a request to the daemon
       (the latter is expected to fail anyway).
    
    For #2, the suggestion is to move the reading of the
       currentContextMap->cancellable flag under the clientMutex lock.
    
       Also it may be advisable to change the SCardGetAndLockContext
       function interface so that it's more difficult to use it in thread-unsafe
       manner: remove the second parameter "int lock" and make the
       locking behavior the default one, and introduce another function
       that does no locks and returns a boolean instead of an
       SCONTEXTMAP* pointer. "
    
    Thanks to Maksim Ivanov for the bug report and solution
    "[Pcsclite-muscle] Data races related to SCardCancel"
    http://lists.alioth.debian.org/pipermail/pcsclite-muscle/Week-of-Mon-20170116/000789.html
---
 src/winscard_clnt.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/winscard_clnt.c b/src/winscard_clnt.c
index 76175e4..98fe61d 100644
--- a/src/winscard_clnt.c
+++ b/src/winscard_clnt.c
@@ -3072,6 +3072,7 @@ LONG SCardCancel(SCARDCONTEXT hContext)
 	LONG rv = SCARD_S_SUCCESS;
 	uint32_t dwClientID = 0;
 	struct cancel_struct scCancelStruct;
+	char cancellable;
 
 	PROFILE_START
 	API_TRACE_IN("%ld", hContext)
@@ -3079,14 +3080,17 @@ LONG SCardCancel(SCARDCONTEXT hContext)
 	/*
 	 * Make sure this context has been opened
 	 */
-	currentContextMap = SCardGetAndLockContext(hContext, FALSE);
+	currentContextMap = SCardGetAndLockContext(hContext, TRUE);
 	if (NULL == currentContextMap)
 	{
 		rv = SCARD_E_INVALID_HANDLE;
 		goto error;
 	}
 
-	if (! currentContextMap->cancellable)
+	cancellable = currentContextMap->cancellable;
+	(void)pthread_mutex_unlock(&currentContextMap->mMutex);
+
+	if (! cancellable)
 	{
 		rv = SCARD_S_SUCCESS;
 		goto error;

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pcsclite/PCSC.git