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

A ZIP 321 URI with filename-unsafe characters (e.g. +) is reportedly accepted when it MUST NOT be according to the spec #1420

Open
daira opened this issue Nov 20, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@daira
Copy link
Contributor

daira commented Nov 20, 2024

ZIP 321 says, of the base64url memo field:

"base64url" is defined as in [4] with padding omitted. (Note that this uses a different alphabet to the usual base64; the values 62 and 63 in the alphabet are encoded as - and _ respectively. Implementations MUST NOT accept the characters +, /, and = that occur only in the usual base64.)

[4] RFC 4648 section 5: Base64 Encoding with URL and Filename Safe Alphabet

According to Electric-Coin-Company/zashi-android#1678 (comment) , a URI with a + character is rejected on Zashi Android but accepted on Zashi iOS. According to the spec, it MUST be rejected on both. (I'm intentionally using the more value-neutral terms "accepted"/"rejected" rather than "succeeds"/"fails".)

In general we do not follow the Postel-era RFC "Be generous in what you accept." dictum when writing or implementing Zcash specs —or at least we don't intend to— because it demonstrably results in significant security and interoperability problems over the long term. For discussion of this see RFC 9413: Maintaining Robust Protocols, originally drafted with less weasel-wording and under the better title "The Harmful Consequences of the Robustness Principle" (which ruffled a few political feathers with the IETF old-timers, sigh).

Among other benefits, such as easier security analysis, that practice was intended to prevent situations such as that encountered in zashi-android#1678: someone testing on one implementation (Zashi iOS) and then getting inconsistent behaviour on another (Zashi Android). In this case interoperability problems could also occur if filename-unsafe characters are mangled by some transmission channels, or inconsistently converted to/from a QR code. If we accepted the wider set of characters then we'd have to test all those cases (or whatever subset of them we were able to think of), and would be imposing a similar testing burden on other implementors.

@daira
Copy link
Contributor Author

daira commented Nov 20, 2024

Example of a QR code that is reportedly accepted when it must not be (second image in Electric-Coin-Company/zashi-android#1678 (comment) ):

image

@pacu
Copy link
Contributor

pacu commented Nov 20, 2024

@daira thanks for reporting this I'll check it out.

If someone can chip in and do it before myself I'd much appreciate it.

@daira
Copy link
Contributor Author

daira commented Nov 21, 2024

The problem is as described here: zecdev/zcash-swift-payment-uri#69 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants