Skip to content

Commit

Permalink
Guard against overflows. Release infos without defer.
Browse files Browse the repository at this point in the history
  • Loading branch information
codingllama committed May 9, 2022
1 parent 6edc672 commit 8148bdf
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 18 deletions.
40 changes: 22 additions & 18 deletions lib/auth/touchid/api_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,53 +157,57 @@ func findCredentialsImpl(rpID, user string, find func(C.LabelFilter, **C.Credent

start := unsafe.Pointer(infosC)
size := unsafe.Sizeof(C.CredentialInfo{})
infos := make([]CredentialInfo, res)
infos := make([]CredentialInfo, 0, res)
for i := 0; i < int(res); i++ {
// IMPORTANT: The defer below is used to free the pointers inside infos.
// It relies on the fact that we never error out of the function after
// this point, otherwise some instances would leak.
infoC := (*C.CredentialInfo)(unsafe.Add(start, uintptr(i)*size))
defer func() {
var label, appLabel, appTag, pubKeyB64 string
{
infoC := (*C.CredentialInfo)(unsafe.Add(start, uintptr(i)*size))

// Get all data from infoC...
label = C.GoString(infoC.label)
appLabel = C.GoString(infoC.app_label)
appTag = C.GoString(infoC.app_tag)
pubKeyB64 = C.GoString(infoC.pub_key_b64)

// ... then free it before proceeding.
C.free(unsafe.Pointer(infoC.label))
C.free(unsafe.Pointer(infoC.app_label))
C.free(unsafe.Pointer(infoC.app_tag))
C.free(unsafe.Pointer(infoC.pub_key_b64))
}()
}

// credential ID / UUID
credentialID := appLabel

// user@rpid
label := C.GoString(infoC.label)
rpID, user := splitLabel(label)
if rpID == "" || user == "" {
log.Debugf("Skipping credential with unexpected label: %q", label)
log.Debugf("Skipping credential %q: unexpected label: %q", credentialID, label)
continue
}

// credential ID / UUID
credentialID := C.GoString(infoC.app_label)

// user handle
appTag := C.GoString(infoC.app_tag)
userHandle, err := base64.RawURLEncoding.DecodeString(appTag)
if err != nil {
log.Debugf("Skipping credential with unexpected application tag: %q", appTag)
log.Debugf("Skipping credential %q: unexpected application tag: %q", credentialID, appTag)
continue
}

// ECDSA public key
pubKeyB64 := C.GoString(infoC.pub_key_b64)
pubKeyRaw, err := base64.StdEncoding.DecodeString(pubKeyB64)
if err != nil {
log.WithError(err).Warnf("Failed to decode public key for credential %q", credentialID)
// Do not return or break here, we want the defers to run in all items.
// Do not return or break out of the loop, it needs to run in order to
// deallocate the structs within.
}

infos[i] = CredentialInfo{
infos = append(infos, CredentialInfo{
UserHandle: userHandle,
CredentialID: credentialID,
RPID: rpID,
User: user,
publicKeyRaw: pubKeyRaw,
}
})
}
return infos, int(res)
}
5 changes: 5 additions & 0 deletions lib/auth/touchid/credentials.m
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#import <Foundation/Foundation.h>
#import <Security/Security.h>

#include <limits.h>
#include <stdlib.h>

#include "common.h"
Expand Down Expand Up @@ -63,6 +64,10 @@ int FindCredentials(LabelFilter filter, CredentialInfo **infosOut) {
NSString *nsFilter = [NSString stringWithUTF8String:filter.value];

CFIndex count = CFArrayGetCount(items);
// Guard against overflows, just in case we ever get that many credentials.
if (count > INT_MAX) {
count = INT_MAX;
}
*infosOut = calloc(count, sizeof(CredentialInfo));
int infosLen = 0;
for (CFIndex i = 0; i < count; i++) {
Expand Down

0 comments on commit 8148bdf

Please sign in to comment.