-
Notifications
You must be signed in to change notification settings - Fork 1k
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
v3: panic "attempted to parse unknown event (please report): none" #666
Comments
Thanks for the report, I'll look into it. |
As another data point, I found this occurs on invalid YAML that includes |
Same issue with YAML above ^ |
This issue was assigned CVE-2022-28948 |
Issue 666. 😈 |
For people who land here from Snyk: they incorrectly marked the vulnerability as being in v2. I've been unable to reproduce the issue in v2, and can only reproduce it in v3. I've notified Snyk of the error. UPDATE: They fixed it! UPDATE 2: For people who land here from the GitHub security advisory: they incorrectly marked the vulnerability as being in v2. Trying to find an appropriate version range to suggest... |
Sorry for being slow. The post mortem is that long ago I was thrown off by an awkward API in the underlying library: it returns success (true) in error cases in that particular function call, while in general it does not. Instead of fixing the underlying API, we'll continue to tolerate it for the time being and handle the issue on the high-level decoder, so that it remains somewhat easy to compare the original libyaml logic to what remains of it in go-yaml. |
@niemeyer @stevebeattie why this crash worth a CVE, and no severity? |
@CityOfLight77 it’s a CVE because a lot of apps use go-yaml. If a malicious user sends one of these payloads to such an app, they can shut down the app (Denial of Service). Snyk assigned a severity of 7.5. The severity really depends on how your app uses go-yaml. If an unauthenticated user can send a malicious payload, then the severity is higher than it would be if they must be authenticated. |
Thanks for the explanation |
@crenshaw-dev thanks for the heads up - we (Snyk) have updated our advisory now to only show versions of yaml v3 as vulnerable, and also included the fix advice released in v3.0.0. That should be live in an hour or so 🙏 |
Thanks all. |
As the dependabot cannot update the go-yaml automatically, this PR updates gopkg.in/yaml.v3 to v3.0.0 to fix CVE-2022-28948 (See go-yaml/yaml#666) Signed-off-by: Yong Tang <[email protected]>
An issue in the Unmarshal function in Go-Yaml v3 causes the program to crash when attempting to deserialize invalid input. https://nvd.nist.gov/vuln/detail/CVE-2022-28948 go-yaml/yaml#666
From the security report: > An issue in the Unmarshal function in Go-Yaml v3 causes the program to crash > when attempting to deserialize invalid input. While upgrading to version 3, there was some required changes: * Force the encoder to use 2 spaces for identation * Rewrite tests so lists are idented References: https://nvd.nist.gov/vuln/detail/CVE-2022-28948 go-yaml/yaml#666 GHSA-hp87-p4gw-j4gq
This fixes a potential panic when the library attempts to unmarshal invalid data. See this issue [0] for details. [0]: go-yaml/yaml#666 Signed-off-by: William Findlay <[email protected]>
This fixes a potential panic when the library attempts to unmarshal invalid data. See this issue [0] for details. [0]: go-yaml/yaml#666 Signed-off-by: William Findlay <[email protected]>
As the dependabot cannot update the go-yaml automatically, this PR updates gopkg.in/yaml.v3 to v3.0.0 to fix CVE-2022-28948 (See go-yaml/yaml#666) Signed-off-by: Yong Tang <[email protected]>
This seems to be correct from my testing too, maybe the OP could be edited to say there is no issue in v2 at the top. The GH advisory that came out on this is misleading people to upgrade I believe. |
@codyoss which GH SA? I'm not seeing one on this repo. |
Per go-yaml/yaml#666, the recommendation is to bump to v3.0.1. I detected this as an indirect dependency that was flagged in net-istio.
Per go-yaml/yaml#666, the recommendation is to bump to v3.0.1. I detected this as an indirect dependency that was flagged in net-istio.
Issue 666 😈 |
Update to newly released v3.0.0 which fixes go-yaml/yaml#666.
Hi @crenshaw-dev, @niemeyer, |
See go-yaml/yaml#666, and go-yaml/yaml#665. Signed-off-by: Alexandre Perrin <[email protected]>
See go-yaml/yaml#666, and go-yaml/yaml#665. Signed-off-by: Alexandre Perrin <[email protected]>
Could somebody help here: if go yaml version in use is v2.4.0 does it mean it is v2? Getting above cve against v2.4.0 :/ |
I believe the answer to your question is "yes". Regarding how the versions map to which branches/series, see these pages for additional details: |
Hi folks 👋🏻 Found this panic (along with #665) while fuzzing my own project.
Minimal example of the panic (https://play.golang.org/p/gLM_eHzcrgz):
Output:
The text was updated successfully, but these errors were encountered: