From 87c80bfba583eadc087810d17aa631ef4e405efc Mon Sep 17 00:00:00 2001
From: Justin Cormack <justin.cormack@docker.com>
Date: Mon, 1 Jul 2019 14:37:24 +0100
Subject: [PATCH] Fix a double free in the List functions

The code was set up so that it would free the individual items and the data
in `freeListData`, but there was already a Go `defer` to free the data item,
resulting in a double free.

Remove the `free` in `freeListData` and leave the original one.

In addition, move the `defer` for freeing the list data before the error
check, so that the data is also free in the error case. This just removes
a minor leak.

This vulnerability was discovered by:
Jasiel Spelman of Trend Micro Zero Day Initiative and Trend Micro Team Nebula

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
---
 osxkeychain/osxkeychain_darwin.c     | 1 -
 osxkeychain/osxkeychain_darwin.go    | 5 ++---
 secretservice/secretservice_linux.c  | 1 -
 secretservice/secretservice_linux.go | 4 ++--
 4 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/osxkeychain/osxkeychain_darwin.c b/osxkeychain/osxkeychain_darwin.c
index f84d61ee..845012bd 100644
--- a/osxkeychain/osxkeychain_darwin.c
+++ b/osxkeychain/osxkeychain_darwin.c
@@ -224,5 +224,4 @@ void freeListData(char *** data, unsigned int length) {
      for(int i=0; i<length; i++) {
         free((*data)[i]);
      }
-     free(*data);
 }
diff --git a/osxkeychain/osxkeychain_darwin.go b/osxkeychain/osxkeychain_darwin.go
index da57d00c..6c8ac571 100644
--- a/osxkeychain/osxkeychain_darwin.go
+++ b/osxkeychain/osxkeychain_darwin.go
@@ -109,6 +109,8 @@ func (h Osxkeychain) List() (map[string]string, error) {
 	defer C.free(unsafe.Pointer(acctsC))
 	var listLenC C.uint
 	errMsg := C.keychain_list(credsLabelC, &pathsC, &acctsC, &listLenC)
+	defer C.freeListData(&pathsC, listLenC)
+	defer C.freeListData(&acctsC, listLenC)
 	if errMsg != nil {
 		defer C.free(unsafe.Pointer(errMsg))
 		goMsg := C.GoString(errMsg)
@@ -119,9 +121,6 @@ func (h Osxkeychain) List() (map[string]string, error) {
 		return nil, errors.New(goMsg)
 	}
 
-	defer C.freeListData(&pathsC, listLenC)
-	defer C.freeListData(&acctsC, listLenC)
-
 	var listLen int
 	listLen = int(listLenC)
 	pathTmp := (*[1 << 30]*C.char)(unsafe.Pointer(pathsC))[:listLen:listLen]
diff --git a/secretservice/secretservice_linux.c b/secretservice/secretservice_linux.c
index 35dea92d..73502a2e 100644
--- a/secretservice/secretservice_linux.c
+++ b/secretservice/secretservice_linux.c
@@ -158,5 +158,4 @@ void freeListData(char *** data, unsigned int length) {
 	for(i=0; i<length; i++) {
 		free((*data)[i]);
 	}
-	free(*data);
 }
diff --git a/secretservice/secretservice_linux.go b/secretservice/secretservice_linux.go
index 383b0c24..26efc0d1 100644
--- a/secretservice/secretservice_linux.go
+++ b/secretservice/secretservice_linux.go
@@ -92,12 +92,12 @@ func (h Secretservice) List() (map[string]string, error) {
 	defer C.free(unsafe.Pointer(acctsC))
 	var listLenC C.uint
 	err := C.list(credsLabelC, &pathsC, &acctsC, &listLenC)
+	defer C.freeListData(&pathsC, listLenC)
+	defer C.freeListData(&acctsC, listLenC)
 	if err != nil {
 		defer C.g_error_free(err)
 		return nil, errors.New("Error from list function in secretservice_linux.c likely due to error in secretservice library")
 	}
-	defer C.freeListData(&pathsC, listLenC)
-	defer C.freeListData(&acctsC, listLenC)
 
 	resp := make(map[string]string)