Skip to content

Commit 7f2666e

Browse files
committed
Reject duplicate build metadata when verifying.
1 parent 9e18d2e commit 7f2666e

1 file changed

Lines changed: 51 additions & 3 deletions

File tree

  • cmd/soroban-cli/src/commands/contract

cmd/soroban-cli/src/commands/contract/verify.rs

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ pub enum Error {
101101
#[error("the WASM's contractmetav0 does not record a `bldimg` entry; cannot verify")]
102102
MissingBldimg,
103103

104+
#[error("the WASM's contractmetav0 records more than one `{field}` entry; refusing to verify (which value applies is ambiguous)")]
105+
DuplicateMeta { field: &'static str },
106+
104107
#[error("the WASM's contractmetav0 does not record a `source_sha256` entry; cannot verify")]
105108
MissingSourceSha256,
106109

@@ -457,14 +460,28 @@ pub fn extract_metadata(wasm: &[u8]) -> Result<ExtractedMetadata, Error> {
457460
let mut source_sha256: Option<String> = None;
458461
let mut bldopts: Vec<String> = Vec::new();
459462

463+
// Each of these SEP-58 fields must appear at most once. Reject duplicates
464+
// rather than silently taking the last: two `bldimg` entries (say a benign
465+
// one to fool inspection and a second the cli would actually trust and
466+
// rebuild in) would be a verification-bypass vector, and the same ambiguity
467+
// applies to the `source_uri`/`source_sha256` that pin what gets rebuilt.
468+
let set_once =
469+
|slot: &mut Option<String>, field: &'static str, v: String| -> Result<(), Error> {
470+
if slot.is_some() {
471+
return Err(Error::DuplicateMeta { field });
472+
}
473+
*slot = Some(v);
474+
Ok(())
475+
};
476+
460477
for entry in &spec.meta {
461478
let ScMetaEntry::ScMetaV0(ScMetaV0 { key, val }) = entry;
462479
let k = key.to_string();
463480
let v = val.to_string();
464481
match k.as_str() {
465-
"bldimg" => bldimg = Some(v),
466-
"source_uri" => source_uri = Some(v),
467-
"source_sha256" => source_sha256 = Some(v),
482+
"bldimg" => set_once(&mut bldimg, "bldimg", v)?,
483+
"source_uri" => set_once(&mut source_uri, "source_uri", v)?,
484+
"source_sha256" => set_once(&mut source_sha256, "source_sha256", v)?,
468485
"bldopt" => bldopts.push(v),
469486
_ => {} // cliver and any user --meta are intentionally ignored
470487
}
@@ -928,6 +945,37 @@ mod tests {
928945
assert!(matches!(err, Error::MissingBldimg));
929946
}
930947

948+
#[test]
949+
fn extract_metadata_duplicate_bldimg_errors() {
950+
// A second bldimg — e.g. a benign one to fool inspection plus one the
951+
// cli would actually trust and rebuild in — must be rejected outright.
952+
let other = format!("docker.io/attacker/evil@sha256:{}", "b".repeat(64));
953+
let wasm = make_wasm_with_meta(&[
954+
("bldimg", &good_bldimg()),
955+
("bldimg", &other),
956+
("source_sha256", &"f".repeat(64)),
957+
]);
958+
let err = extract_metadata(&wasm).unwrap_err();
959+
assert!(matches!(err, Error::DuplicateMeta { field: "bldimg" }));
960+
}
961+
962+
#[test]
963+
fn extract_metadata_duplicate_source_ids_error() {
964+
for field in ["source_uri", "source_sha256"] {
965+
let wasm = make_wasm_with_meta(&[
966+
("bldimg", &good_bldimg()),
967+
("source_sha256", &"f".repeat(64)),
968+
(field, "https://example.com/a.tar.gz"),
969+
(field, "https://example.com/b.tar.gz"),
970+
]);
971+
let err = extract_metadata(&wasm).unwrap_err();
972+
assert!(
973+
matches!(err, Error::DuplicateMeta { field: f } if f == field),
974+
"expected DuplicateMeta for {field}, got {err:?}"
975+
);
976+
}
977+
}
978+
931979
#[test]
932980
fn extract_metadata_missing_source_id_errors() {
933981
let wasm = make_wasm_with_meta(&[("bldimg", &good_bldimg())]);

0 commit comments

Comments
 (0)