Potential Memory Leak - fido_credman_get_dev_rp()
NoMoreFood opened this issue · 4 comments
What version of libfido2 are you using?
libfido2-1.10.0 (64-Bit)
What operating system are you running?
Windows 11
What application are you using in conjunction with libfido2?
Custom Code (C++)
How does the problem manifest itself?
Visual Studio seems to report significant number of memory leaks after using the fido_credman_get_dev_rp() function, and I believe I am doing the cleanup properly. I provided one simplified example below. Note other functions fido_dev_info_manifest() also appear to leak a little due to some lists it internally maintains but fido_credman_get_dev_rp() is much worse. Any insight would be appreciated.
Is the problem reproducible?
Yes
What are the steps that lead to the problem?
#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>
#include <fido.h>
#include <fido/credman.h>
#include <fido/eddsa.h>
#pragma comment(lib,"crypto-47.lib")
#pragma comment(lib,"fido2.lib")
#pragma comment(lib,"cbor.lib")
#pragma comment(lib,"zlib.lib")
#pragma comment(lib, "SetupAPI.lib")
#pragma comment(lib, "Hid.lib")
int wmain(int argc, char ** argv)
{
_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
const char* sPin = "XXXX";
fido_dev_info* tDevList = fido_dev_info_new(64);
if (tDevList == NULL) return 0;
// enumerate devices
size_t iDevices;
fido_dev_info_manifest(tDevList, 64, &iDevices);
for (size_t iDevice = 0; iDevice < iDevices; iDevice++)
{
const fido_dev_info_t* tDeviceInfo = fido_dev_info_ptr(tDevList, iDevice);
// connect to the device
fido_dev_t* tDevice = fido_dev_new();
if (tDevice == NULL) continue;
if (fido_dev_open(tDevice, fido_dev_info_path(tDeviceInfo)) == FIDO_OK)
{
// get relying party list for this device
fido_credman_rp_t* tRelyingParty = fido_credman_rp_new();
if (tRelyingParty != NULL && fido_credman_get_dev_rp(tDevice, tRelyingParty, sPin) == FIDO_OK)
{
// more stuff ...
}
fido_credman_rp_free(&tRelyingParty);
fido_dev_close(tDevice);
}
fido_dev_free(&tDevice);
}
fido_dev_info_free(&tDevList, 64);
return 0;
}
Produces:
Detected memory leaks!
Dumping objects ->
{2633} normal block at 0x000001F3024B2F60, 24 bytes long.
Data: <@=K > 40 3D 4B 02 F3 01 00 00 00 00 00 00 00 00 00 00
{2632} normal block at 0x000001F3024B42E0, 32 bytes long.
Data: < > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
{2631} normal block at 0x000001F3024B2BA0, 32 bytes long.
Data: < BK > 00 00 00 00 CD CD CD CD E0 42 4B 02 F3 01 00 00
{2630} normal block at 0x000001F3024B3D40, 24 bytes long.
Data: < +K > 0D 00 00 00 CD CD CD CD A0 2B 4B 02 F3 01 00 00
{2628} normal block at 0x000001F302497260, 40 bytes long.
Data: < > FF FF FF FF FF FF FF FF FF FF FF FF 00 00 00 00
<<<<<many, many more lines -- truncated>>>>>
Object dump complete.
Does the problem happen with different authenticators?
N/A
Please include the output of fido2-token -L.
N/A
Please include the output of fido2-token -I.
N/A
Hi, thank you for the report. I adapted the code sample to run on Linux and had a quick look using Valgrind. I can confirm the fido_dev_info_manifest() leak (known problem), but that's all I see, which makes me wonder if the other leaks could be in Windows-specific code (hid_win.c).
ted:~$ cat x.cc
#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
//#include <crtdbg.h>
#include <fido.h>
#include <fido/credman.h>
#include <fido/eddsa.h>
#if 0
#pragma comment(lib,"crypto-47.lib")
#pragma comment(lib,"fido2.lib")
#pragma comment(lib,"cbor.lib")
#pragma comment(lib,"zlib.lib")
#pragma comment(lib, "SetupAPI.lib")
#pragma comment(lib, "Hid.lib")
#endif
int run(void)
{
//_CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
const char* sPin = "XXX";
fido_dev_info* tDevList = fido_dev_info_new(64);
if (tDevList == NULL) return 0;
// enumerate devices
size_t iDevices;
fido_dev_info_manifest(tDevList, 64, &iDevices);
for (size_t iDevice = 0; iDevice < iDevices; iDevice++)
{
const fido_dev_info_t* tDeviceInfo = fido_dev_info_ptr(tDevList, iDevice);
// connect to the device
fido_dev_t* tDevice = fido_dev_new();
if (tDevice == NULL) continue;
if (fido_dev_open(tDevice, fido_dev_info_path(tDeviceInfo)) == FIDO_OK)
{
// get relying party list for this device
fido_credman_rp_t* tRelyingParty = fido_credman_rp_new();
if (tRelyingParty != NULL && fido_credman_get_dev_rp(tDevice, tRelyingParty, sPin) == FIDO_OK)
{
for (size_t i = 0; i < fido_credman_rp_count(tRelyingParty); i++)
{
printf("%zu: %s\n", i, fido_credman_rp_id(tRelyingParty, i));
}
}
fido_credman_rp_free(&tRelyingParty);
fido_dev_close(tDevice);
}
fido_dev_free(&tDevice);
}
fido_dev_info_free(&tDevList, 64);
return 0;
}
int main(void)
{
run();
}
ted:~$ g++ -o x x.cc -L/home/pedro/tmp/lib64 -Wl,-rpath=/home/pedro/tmp/lib64 -lfido2
ted:~$ ldd x | grep fido2
libfido2.so.1 => /home/pedro/tmp/lib64/libfido2.so.1 (0x00007fe398708000)
ted:~$ valgrind --leak-check=full --show-leak-kinds=all -s ./x
==19813== Memcheck, a memory error detector
==19813== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==19813== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==19813== Command: ./x
==19813==
==19813==
==19813== HEAP SUMMARY:
==19813== in use at exit: 32 bytes in 2 blocks
==19813== total heap usage: 2,116 allocs, 2,114 frees, 585,598 bytes allocated
==19813==
==19813== 16 bytes in 1 blocks are still reachable in loss record 1 of 2
==19813== at 0x4849464: calloc (vg_replace_malloc.c:1328)
==19813== by 0x486FC6D: fido_dev_register_manifest_func (dev.c:269)
==19813== by 0x486FDD5: fido_dev_info_manifest (dev.c:307)
==19813== by 0x40124B: run() (in /home/pedro/x)
==19813== by 0x40139A: main (in /home/pedro/x)
==19813==
==19813== 16 bytes in 1 blocks are still reachable in loss record 2 of 2
==19813== at 0x4849464: calloc (vg_replace_malloc.c:1328)
==19813== by 0x486FC6D: fido_dev_register_manifest_func (dev.c:269)
==19813== by 0x486FDF2: fido_dev_info_manifest (dev.c:310)
==19813== by 0x40124B: run() (in /home/pedro/x)
==19813== by 0x40139A: main (in /home/pedro/x)
==19813==
==19813== LEAK SUMMARY:
==19813== definitely lost: 0 bytes in 0 blocks
==19813== indirectly lost: 0 bytes in 0 blocks
==19813== possibly lost: 0 bytes in 0 blocks
==19813== still reachable: 32 bytes in 2 blocks
==19813== suppressed: 0 bytes in 0 blocks
==19813==
==19813== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Interesting. Time permitting, I may try to build the library myself and investigate on the Windows side -- I just didn't want to dive in if this was a known issue, if I was doing something errant in my code, or if someone else more familiar with the code could see anything obvious.
Narrowing it down for fido_credman_get_dev_rp(). The allocation appears to originate from a few libressl functions so either it's leaking internally (by design or accidentally) or libfido isn't cleaning up after using libressl.
Alright, done enough research at this point that it's "by design" internally with the SSL libraries and there's no great way to cleanup. Given the problem does not appear to live within libfido2, I'm closing this.