Add advisory for executable in serde_derive crate

[mulkieran]

No description provided.

[ttf]

1.0.171 is still unaffected. The change was introduced about a week after its release in serde-rs/serde@9e8f148

[amousset]

There are no known vulnerabilities in the serde_derive crate linked to this change (i.e. the embedded binary is presumably not malicious nor open to attacks), so IMO this can only be an informational advisory at most. Furthermore, there are AFAIK no official policies preventing or even advising against embedding binaries in crates on crates.io, and RustSec is not the place to settle this debate.

On the other hand it causes important concern (that I share), especially regarding supply-chain security consequences, in the linked issue. So I think it makes sense to issue a notice advisory, not to disapprove or approve the change but to notify people who might be impacted (I think the advisory wording matches this goal), and that the security aspect of it makes it relevant for RustSec.

[Shnatsel]

I think it is important to check how this information is surfaced before we merge this.

This is a very popular crate, so if this starts failing CI (or even just creating extra noise for people), the blast radius will be very large.

This includes tooling we do not directly control, such as OSV export (and tools consuming it) and GHSA (including Dependabot).

[amousset]

I'll work on this tomorrow (and more generally on a general document about how our advisories are treated depending on their properties in the various tooling consuming them).

At first sight, it looks like our latest notice advisory was imported into OSV (with nothing identifying it as informational except in the database-specific part), but not into GitHub database. So it would likely trigger a vulnerability report at least with osv-scanner, trivy, etc. Hence such an advisory does not actually look currently acceptable in this regard.

[Shnatsel]

Thanks for checking. We could exclude notice type advisories from the OSV export to work around that.

[jhpratt]

(i.e. the embedded binary is presumably not malicious nor open to attacks)

Frankly, I don't think there should be a presumption of anything here. The binary has not been reproduced yet, so we don't know what's actually in the binary.

[amousset]

Thanks for checking. We could exclude notice type advisories from the OSV export to work around that.

Yes, we could actually split the "OSV-formatted export of whole RustSec" from the "RustSec export for import into the osv.dev database". We would need to check hwo osv.dev handles advisories disappearing from the source (we probably don't want to keep https://osv.dev/vulnerability/RUSTSEC-2022-0058 with a dead source source link...)

[amousset]

Frankly, I don't think there should be a presumption of anything here. The binary has not been reproduced yet, so we don't know what's actually in the binary.

I understand it may seem restrictive, but until we know something is actually vulnerable or malicious, I don't think it should be a vulnerability advisory (i.e. not informational) in RustSec.

If you have doubts about the content of a crate being malicious, you should report it to security@rudder.io according to the project's security policy, not to RustSec (until the vulnerability is made public).
If you think binary programs should be forbidden in crate source published in crates.io, it should probably be decided through the RFC process.

IMHO RustSec is not the place to settle this.

[kayabaNerve]

"serde_derive downloads and executes unknown binary" is enough for me to consider this a security issue.

Considering we don't have a formal court, and RustSec is meant to make people aware of security issues, and any binary blobs spits in the face of supply chain security, I'd argue RustSec SHOULD issue an advisory. I'd hope for one stronger than informational until the binary is verifiable, and then I'd argue for an informational class.

[ryanavella]

"serde_derive downloads and executes unknown binary" is enough for me to consider this a security issue.

I'd say unreproducible instead of unknown, see serde-rs/serde#2575.

I have been attempting to follow the provided instructions but have not yet successfully reproduced it.

[LikeLakers2]

I have been attempting to follow the provided instructions but have not yet successfully reproduced it.

I feel like the advisory should mention this, then? I have no idea how you might handle the advisory once the executable becomes reproducible though (assuming it does).

Apologies if I'm misunderstanding how advisories work, by the way.

[ssokolow]

I have no idea how you might handle the advisory once the executable becomes reproducible though (assuming it does).

Wouldn't you just push a revision to the advisory? advisory-db isn't the crates.io package store. The advisories aren't immutable.

[LikeLakers2]

It appears this issue has been solved in serde_derive v1.0.184 - the crate no longer bundles an executable.

---

I have no idea how you might handle the advisory once the executable becomes reproducible though (assuming it does).

Wouldn't you just push a revision to the advisory? advisory-db isn't the crates.io package store. The advisories aren't immutable.

Yes, but there were concerns about how third-parties consume the RustSec advisories within this issue - hence why I was hesitant to suggest a simple revision to the notice, as I'm unsure if third-parties would pick up the changes.

[ssokolow]

Ahh. That makes sense.

[tarcieri]

Given the executable has been removed in new releases and that the previous binary has been mostly-reproduced with only innocuous discrepancies between the packaged version and the one compiled by source, I'm inclined to close this issue.

[alex]

+1 from me to closing.

[Shnatsel]

+1 to closing.

[LikeLakers2]

The executable still exists in old published versions, though - shouldn't the advisory be made for those versions?

[tarcieri]

The executables have been inspected and found to be innocuous.

If a malicious executable is discovered, we can make an advisory for that.

[ssokolow]

It still broke certain build processes (I think cargo2nix was cited as one) and, as such, has been demonstrated to be a footgun people might want to be warned about.

[mulkieran]

Well, I updated the patched table entry, in case that helps: mulkieran@66be030 .

[ChrisDenton]

It still broke certain build processes (I think cargo2nix was cited as one) and, as such, has been demonstrated to be a footgun people might want to be warned about.

To my mind, breaking build processes and other such footguns aren't a security issue so it's out of scope for rustsec.

[ssokolow]

That's fair. As long as it's clear why it's out of scope.

[mulkieran]

It still broke certain build processes (I think cargo2nix was cited as one) and, as such, has been demonstrated to be a footgun people might want to be warned about.

To my mind, breaking build processes and other such footguns aren't a security issue so it's out of scope for rustsec.

That rule would definitely exclude the "unmaintained" category as something that rustsec is responsible for.

[alex]

There's a clear nexus between lack of a maintainer and security issues, namely: absent maintenance, a project will not fix (or likely identify) security issues.

[mulkieran]

There's a clear nexus between lack of a maintainer and security issues, namely: absent maintenance, a project will not fix (or likely identify) security issues.

There's merely a possibly increased probability of true security problems. What I'm saying is that an unresponsive or inactive maintainer is no more certainly security-connected than the executable-present problem, taken generally. There is a possibly increased probability of true security problems, for either.

[LikeLakers2]

The way I see it, an executable being in a crate where it doesn't need to be, is just as much of a security issue as an unmaintained library. That is to say, neither case directly implies a security issue, but both introduce uncertainty as to the safety of the crate.

To be clear, I still accept tarcieri's answer on the matter because, in this case, the executables were found to be safe. But I also think that if one topic is a concern, then the other topic should be a concern as well.

[ChrisDenton]

Note: I was responding to the comment that it "broke build processes". That is not in itself security related. There can be many ways in which a crate my break assumptions made by certain build processes.

[mulkieran]

Note: I was responding to the comment that it "broke build processes". That is not in itself security related. There can be many ways in which a crate my break assumptions made by certain build processes.

AFAIU the crate broke a security-related rule in a bunch of distributions and packagers had to seek work-arounds[1][2]. These rules are generally based on the assumption that a crate that packages an executable is not secure. And this brings the subject right back to security again.

In any case, the particular problem is resolved. But the general concern still remains.

[1] https://src.fedoraproject.org/rpms/rust-serde_derive/c/689eea59064df8f2f15240c7464a91349e2d78d4?branch=rawhide
[2] https://src.fedoraproject.org/rpms/stratisd/c/928d848180d054ca81766bff4247dda192a33fc8?branch=rawhide