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

encoding/asn1: Marshal/Unmarshal will create/parse invalid SETs #39036

Open
rolandshoemaker opened this issue May 13, 2020 · 1 comment
Open
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@rolandshoemaker
Copy link
Member

rolandshoemaker commented May 13, 2020

Marshal and Unmarshal will happily encode and parse invalid SETs. Every component of a SET must have a distinct tag, either by using distinct types of each field or using explicit tagging to get around having multiple non-distinct fields. This specification of SETs allows for undefined ordering of components, since in theory each component can only map to a single field.

Currently Marshal ignores this and will happily encode a SET with multiple identical component tags. Similarly Unmarshal will parse an invalid SET. Due to how parsing works though (SETs are parsed in order, the same as SEQUENCEs, so unordered SETs with distinct fields cannot be parsed, see #19873) enforcing this constraint wouldn't provide significant benefit.

I took a stab at this in https://go-review.googlesource.com/c/go/+/233199, but realized that the naive approach of just using the field type was not sufficient. Really the struct test would need to take the final tag for a field into account, after explicit tagging etc has been applied, but unfortunately there is a bunch of logic that happens outside of getUniversalType for determining that which makes it hard to copy into a separate method.

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 18, 2020
@cagedmantis cagedmantis added this to the Backlog milestone May 18, 2020
@cagedmantis
Copy link
Contributor

/cc @FiloSottile @agl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

2 participants