Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate JSON keys create misleading verification results in the web UI #26

Open
DavidBuchanan314 opened this issue Dec 3, 2023 · 1 comment

Comments

@DavidBuchanan314
Copy link

DavidBuchanan314 commented Dec 3, 2023

During a conversation, I was sent this example message: https://cyphr.me/coze#?input={%22pay%22:{%22msg%22:%22Hello%20Retr0id!%22,%22alg%22:%22ES256%22,%22iat%22:1701603747,%22tmb%22:%221KGZzsqiFAYE5uDO3CCh3PMl9pqwlG1RrI8i5gZY94c%22,%22typ%22:%22cyphr.me/msg/create%22},%22sig%22:%22vZEALUQShRlLYyhJoGmkxpQhhEeUPlzbIZqyTsUyPBMlJMIqGClgSs3uXTDiwumsMivFQKU8K4z4Ec7WlxYIew%22}&dontSignRevoke&updateIat&selectedAlg=ES256&verify

By introducing a duplicate "msg" key, I was able to forge a new message that also passes signature verification according to the web UI: https://cyphr.me/coze#?input={%22pay%22:{%22msg%22:%22Hello,%20Zamicol.%20I%20believe%20you%20will%20find%20that%20this%20Coze%20message%20is%20also%20(supposedly)%20signed%20by%20your%20key!%20Coze%20could%20fix%20this%20issue%20with%20better%20UI,%20but%20I%20think%20this%20illustrates%20just%20how%20hard%20it%20is%20to%20canonicalize%20JSON.%20This%20wouldn't%20be%20possible%20in%20the%20first%20place%20if%20you%20were%20signing%20base64'd%20bytes.%22,%22msg%22:%22Hello%20Retr0id!%22,%22alg%22:%22ES256%22,%22iat%22:1701603747,%22tmb%22:%221KGZzsqiFAYE5uDO3CCh3PMl9pqwlG1RrI8i5gZY94c%22,%22typ%22:%22cyphr.me/msg/create%22},%22sig%22:%22vZEALUQShRlLYyhJoGmkxpQhhEeUPlzbIZqyTsUyPBMlJMIqGClgSs3uXTDiwumsMivFQKU8K4z4Ec7WlxYIew%22}&dontSignRevoke&updateIat&selectedAlg=ES256&verify

The web UI gives no indication that there's anything awry here, and proclaims that the message was verified.

image

This would be a non-issue in many use-cases, because any code reading the msg parameter will likely see only the real one - but an implementation in some other language may see only the first, causing breakage. But, for the purposes of the web UI, it's definitely misleading.

As an aside, I think Coze's general design/approach is fine, but dealing with JSON like this is error-prone, and here is one such error - one which is hopefully an easy fix.

@zamicol
Copy link
Contributor

zamicol commented Dec 5, 2023

Thank you, DavidBuchanan314, for the report. I appreciate that you spent the time to write this up.

The bug is in the CozeJS Verifier repository. https://github.com/Cyphrme/CozeJS

The problem is that CozeJS operates on Javascript objects. In ES5 it should work as expected. There was a change to ES6 that now causes this behavior. This just means that the verifier needs to check JSON objects for duplicates before they become JavaScript object.

It's disappointing to see this as we wrote tests to avoid this very issue. See the live test here: https://cyphrme.github.io/CozeJS/verifier/browsertest/browsertest.html

See the test function test_Duplciate which has notes on this issue. The test is just not correctly testing for this case.

I got most of the fix done on my machine and I'm just polishing code. I plan on pushing a fix up later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants