From ac7e34522a4f1847d280250b230400008eebc87d Mon Sep 17 00:00:00 2001 From: Pol Henarejos Date: Mon, 5 Jan 2026 12:33:35 +0100 Subject: [PATCH] Fixed resident credential storage when two userId have the same prefix. Added a specific test for this case. Fixes #241. Signed-off-by: Pol Henarejos --- src/fido/credential.c | 2 +- tests/pico-fido/test_022_discoverable.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/fido/credential.c b/src/fido/credential.c index 6bc479a..645417d 100644 --- a/src/fido/credential.c +++ b/src/fido/credential.c @@ -319,7 +319,7 @@ int credential_store(const uint8_t *cred_id, size_t cred_id_len, const uint8_t * credential_free(&rcred); continue; } - if (memcmp(rcred.userId.data, cred.userId.data, MIN(rcred.userId.len, cred.userId.len)) == 0) { + if (rcred.userId.len == cred.userId.len && memcmp(rcred.userId.data, cred.userId.data, rcred.userId.len) == 0) { sloti = i; credential_free(&rcred); new_record = false; diff --git a/tests/pico-fido/test_022_discoverable.py b/tests/pico-fido/test_022_discoverable.py index 36b7055..92596de 100644 --- a/tests/pico-fido/test_022_discoverable.py +++ b/tests/pico-fido/test_022_discoverable.py @@ -202,10 +202,29 @@ def test_rk_with_allowlist_of_different_rp(resetdevice): assert e.value.code == CtapError.ERR.NO_CREDENTIALS +def test_same_prefix_userId(device): + """ + A make credential request with two different UserIds that share the same prefix should NOT overwrite. + """ + rp = {"id": "sameprefix.org", "name": "Example"} + user1 = {"id": b"user_12", "name": "A fixed name", "displayName": "A fixed display name"} + user2 = {"id": b"user_123", "name": "A fixed name", "displayName": "A fixed display name"} + + mc_res1 = device.MC(rp = rp, options={"rk":True}, user = user1) + + # Should not overwrite the first credential. + mc_res2 = device.MC(rp = rp, options={"rk":True}, user = user2) + + ga_res = device.GA(rp_id=rp['id'])['res'] + + assert ga_res.number_of_credentials == 2 + + def test_same_userId_overwrites_rk(resetdevice): """ A make credential request with a UserId & Rp that is the same as an existing one should overwrite. """ + resetdevice.reset() rp = {"id": "overwrite.org", "name": "Example"} user = generate_random_user() @@ -249,6 +268,7 @@ def test_returned_credential(device): ga_res = device.GA(allow_list=allow_list,options={'up':False})['res'] + print(ga_res) # No other credentials should be returned with pytest.raises(CtapError) as e: