search

peterbourgon / ff

Flags-first package for configuration
Apache License 2.0
1.34k stars 59 forks source link

Update yaml.v2 and go-toml modules #94

Closed decke closed 2 years ago

decke commented 2 years ago

Generated with:

go get -u ./...
go mod tidy
go test

Builds fine and tests run fine as well. Runtime testing was limited to my own use case.

This also brings yaml.v2 to a version which is not affected by CVE-2019-11254 anymore. For details also see: https://github.com/advisories/GHSA-wxc4-f4m6-wwqv

peterbourgon commented 2 years ago

So, if I understand correctly, the issue was excessive resource consumption from pathological .yaml input?

decke commented 2 years ago

Yes, that is also what I understand. Don't know exactly how excessive though. But it's nothing to freak out.

decke commented 2 years ago

Some people might see this because of the GitHub Advistory for CVE-2022-28948 in yaml.v3. Looks like yaml.v2 is not affected according to upstream discussions. So yaml.v2 is okay, yaml.v3 3.0.1 is also okay.

https://github.com/go-yaml/yaml/issues/666

decke commented 2 years ago

For reference I also tried to update ff to yaml.v3 3.0.1 and it works fine as well. Please let me know if you prefer that and I can update the PR if you want.

peterbourgon commented 2 years ago

I'm not really a fan of automatic dependency updates in libraries like ff, because unless that update solves a specific problem, I don't think it accomplishes anything. Go permits multiple major versions of modules in a single binary, so if ff requires v2 and your application requires v3 there's no problem. And within a single major version, the ultimate decision is made by the application, so if ff requires v2.1.0 and your application requires v2.5.0 then you get v2.5.0.

With that said, I appreciate the thought and effort!

decke commented 2 years ago

I know all of this but it is not an effort to do automatic updates. The reasons behind it are to avoid known vulnerabilities.

peterbourgon commented 2 years ago

I know all of this but it is not an effort to do automatic updates. The reasons behind it are to avoid known vulnerabilities.

My understanding was that your linked CVEs don't actually apply to the yaml and toml dependencies used by this module?

I mean, no big deal in this case.

decke commented 2 years ago

Thanks for merging.