1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
|
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>
Origin: upstream, https://github.com/docker/docker-credential-helpers/commit/87c80bf
---
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(-)
--- a/osxkeychain/osxkeychain_darwin.c
+++ b/osxkeychain/osxkeychain_darwin.c
@@ -223,6 +223,5 @@
void freeListData(char *** data, unsigned int length) {
for(int i=0; i<length; i++) {
free((*data)[i]);
}
- free(*data);
}
--- a/osxkeychain/osxkeychain_darwin.go
+++ b/osxkeychain/osxkeychain_darwin.go
@@ -109,17 +109,16 @@
var acctsC **C.char
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)
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]
acctTmp := (*[1 << 30]*C.char)(unsafe.Pointer(acctsC))[:listLen:listLen]
--- a/secretservice/secretservice_linux.c
+++ b/secretservice/secretservice_linux.c
@@ -157,6 +157,5 @@
int i;
for(i=0; i<length; i++) {
free((*data)[i]);
}
- free(*data);
}
--- a/secretservice/secretservice_linux.go
+++ b/secretservice/secretservice_linux.go
@@ -91,14 +91,14 @@
var acctsC **C.char
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)
listLen := int(listLenC)
|
