Skip to content

Commit

Permalink
fix: Additional slice OOB read mitigation.
Browse files Browse the repository at this point in the history
  • Loading branch information
@cdmurph32
cdmurph32 committed Dec 9, 2024
1 parent 5f3d903 commit 1bd44af
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 79 deletions.
40 changes: 31 additions & 9 deletions sdk/src/asset_handlers/jpeg_io.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,9 @@ impl CAIWriter for JpegIO {
if seg > 1 {
// the LBox and TBox are already in the JUMBF
// but we need to duplicate them in all other segments
let lbox_tbox = &store_bytes[..8];
let lbox_tbox = store_bytes
.get(0..8)
.ok_or(Error::InvalidAsset("Store bytes too short".to_string()))?;
seg_data.extend(lbox_tbox);
}
if seg_chucks.len() > 0 {
Expand Down Expand Up @@ -383,7 +385,12 @@ impl CAIWriter for JpegIO {
positions.push(v);
} else {
// check if this is a CAI JUMBF block
let jumb_type = raw_vec.as_mut_slice()[24..28].to_vec();
let jumb_type = raw_vec
.get(24..28)
.ok_or(Error::InvalidAsset(
"Invalid JPEG CAI JUMBF block".to_string(),
))?
.to_vec();
let is_cai = vec_compare(&C2PA_MARKER, &jumb_type);
if is_cai {
cai_seg_cnt = 1;
Expand Down Expand Up @@ -800,11 +807,22 @@ fn make_box_maps(input_stream: &mut dyn CAIRead) -> Result<Vec<BoxMap>> {
if cai_seg_cnt > 0 && is_cai_continuation {
cai_seg_cnt += 1;

let cai_bm = &mut box_maps[cai_index];
cai_bm.range_len += raw_bytes.len() + 4;
if let Some(cai_bm) = box_maps.get_mut(cai_index) {
cai_bm.range_len += raw_bytes.len() + 4;
} else {
return Err(Error::InvalidAsset(
"CAI segment index out of bounds".to_string(),
));
}
} else {
// check if this is a CAI JUMBF block
let jumb_type = raw_vec.as_mut_slice()[24..28].to_vec();
let jumb_type = raw_vec
.as_mut_slice()
.get(24..28)
.ok_or(Error::InvalidAsset(
"Invalid JPEG CAI JUMBF block".to_string(),
))?
.to_vec();
let is_cai = vec_compare(&C2PA_MARKER, &jumb_type);
if is_cai {
cai_seg_cnt = 1;
Expand Down Expand Up @@ -998,13 +1016,15 @@ impl AssetBoxHash for JpegIO {
let mut box_maps = make_box_maps(input_stream)?;

for bm in box_maps.iter_mut() {
if bm.names[0] == C2PA_BOXHASH {
continue;
if let Some(name) = bm.names.first() {
if name == C2PA_BOXHASH {
continue;
}
}

input_stream.seek(std::io::SeekFrom::Start(bm.range_start as u64))?;

let size = if bm.names[0] == "SOS" {
let size = if bm.names.first().map_or(false, |name| name == "SOS") {
let mut size = get_seg_size(input_stream)?;

input_stream.seek(std::io::SeekFrom::Start((bm.range_start + size) as u64))?;
Expand Down Expand Up @@ -1056,7 +1076,9 @@ impl ComposedManifestRef for JpegIO {
if seg > 1 {
// the LBox and TBox are already in the JUMBF
// but we need to duplicate them in all other segments
let lbox_tbox = &manifest_data[..8];
let lbox_tbox = manifest_data
.get(0..8)
.ok_or(Error::InvalidAsset("Manifest data too short".to_string()))?;
seg_data.extend(lbox_tbox);
}
if seg_chucks.len() > 0 {
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/callback_signer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl CallbackSigner {
// Parse the PEM data to get the private key
let pem = parse(private_key).map_err(|e| Error::OtherError(Box::new(e)))?;
// For Ed25519, the key is 32 bytes long, so we skip the first 16 bytes of the PEM data
let key_bytes = &pem.contents()[16..];
let key_bytes = pem.contents().get(16..).ok_or(Error::InvalidSigningKey)?;
let signing_key =
SigningKey::try_from(key_bytes).map_err(|e| Error::OtherError(Box::new(e)))?;
// Sign the data
Expand Down
3 changes: 3 additions & 0 deletions sdk/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,9 @@ pub enum Error {
#[error("unknown algorithm")]
UnknownAlgorithm,

#[error("invalid signing key")]
InvalidSigningKey,

// --- third-party errors ---
#[error(transparent)]
Utf8Error(#[from] std::str::Utf8Error),
Expand Down
117 changes: 67 additions & 50 deletions sdk/src/jumbf/boxes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,67 +282,74 @@ impl JUMBFSuperBox {
self.data_boxes.len()
}

pub fn data_box(&self, index: usize) -> &dyn BMFFBox {
self.data_boxes[index].as_ref()
pub fn data_box(&self, index: usize) -> Option<&dyn BMFFBox> {
self.data_boxes.get(index).map(|b| b.as_ref())
}

pub fn data_box_as_superbox(&self, index: usize) -> Option<&JUMBFSuperBox> {
let da_box = &self.data_boxes[index];
da_box.as_ref().as_any().downcast_ref::<JUMBFSuperBox>()
self.data_boxes
.get(index)
.and_then(|da_box| da_box.as_ref().as_any().downcast_ref::<JUMBFSuperBox>())
}

pub fn data_box_as_json_box(&self, index: usize) -> Option<&JUMBFJSONContentBox> {
let da_box = &self.data_boxes[index];
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFJSONContentBox>()
self.data_boxes.get(index).and_then(|da_box| {
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFJSONContentBox>()
})
}

pub fn data_box_as_cbor_box(&self, index: usize) -> Option<&JUMBFCBORContentBox> {
let da_box = &self.data_boxes[index];
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFCBORContentBox>()
self.data_boxes.get(index).and_then(|da_box| {
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFCBORContentBox>()
})
}

pub fn data_box_as_jp2c_box(&self, index: usize) -> Option<&JUMBFCodestreamContentBox> {
let da_box = &self.data_boxes[index];
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFCodestreamContentBox>()
self.data_boxes.get(index).and_then(|da_box| {
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFCodestreamContentBox>()
})
}

pub fn data_box_as_uuid_box(&self, index: usize) -> Option<&JUMBFUUIDContentBox> {
let da_box = &self.data_boxes[index];
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFUUIDContentBox>()
self.data_boxes.get(index).and_then(|da_box| {
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFUUIDContentBox>()
})
}

pub fn data_box_as_embedded_file_content_box(
&self,
index: usize,
) -> Option<&JUMBFEmbeddedFileContentBox> {
let da_box = &self.data_boxes[index];
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFEmbeddedFileContentBox>()
self.data_boxes.get(index).and_then(|da_box| {
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFEmbeddedFileContentBox>()
})
}

pub fn data_box_as_embedded_media_type_box(
&self,
index: usize,
) -> Option<&JUMBFEmbeddedFileDescriptionBox> {
let da_box = &self.data_boxes[index];
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFEmbeddedFileDescriptionBox>()
self.data_boxes.get(index).and_then(|da_box| {
da_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFEmbeddedFileDescriptionBox>()
})
}
}

Expand Down Expand Up @@ -1432,8 +1439,11 @@ impl CAIStore {
self.store.data_boxes.len()
}

pub fn data_box(&self, index: usize) -> &dyn BMFFBox {
self.store.data_boxes[index].as_ref()
pub fn data_box(&self, index: usize) -> Option<&dyn BMFFBox> {
self.store
.data_boxes
.get(index)
.map(|da_box| da_box.as_ref())
}

pub fn assertion_store(&self) -> Option<&JUMBFSuperBox> {
Expand Down Expand Up @@ -1506,13 +1516,18 @@ impl Cai {
self.sbox.data_boxes.len()
}

pub fn data_box(&self, index: usize) -> &dyn BMFFBox {
self.sbox.data_boxes[index].as_ref()
pub fn data_box(&self, index: usize) -> Option<&dyn BMFFBox> {
self.sbox
.data_boxes
.get(index)
.map(|da_box| da_box.as_ref())
}

pub fn data_box_as_superbox(&self, index: usize) -> Option<&JUMBFSuperBox> {
let da_box = &self.sbox.data_boxes[index];
da_box.as_ref().as_any().downcast_ref::<JUMBFSuperBox>()
self.sbox
.data_boxes
.get(index)
.and_then(|da_box| da_box.as_ref().as_any().downcast_ref::<JUMBFSuperBox>())
}

pub fn store(&self) -> Option<&JUMBFSuperBox> {
Expand Down Expand Up @@ -1575,19 +1590,21 @@ impl JumbfEmbeddedFileBox {
}

pub fn media_type_box(&self) -> Option<&JUMBFEmbeddedFileDescriptionBox> {
let efd_box = &self.embedding_box.data_boxes[0];
efd_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFEmbeddedFileDescriptionBox>()
self.embedding_box.data_boxes.first().and_then(|efd_box| {
efd_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFEmbeddedFileDescriptionBox>()
})
}

pub fn data_box(&self) -> Option<&JUMBFEmbeddedFileContentBox> {
let efc_box = &self.embedding_box.data_boxes[1];
efc_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFEmbeddedFileContentBox>()
self.embedding_box.data_boxes.get(1).and_then(|efc_box| {
efc_box
.as_ref()
.as_any()
.downcast_ref::<JUMBFEmbeddedFileContentBox>()
})
}

pub fn set_salt(&mut self, salt: Vec<u8>) -> JumbfParseResult<()> {
Expand Down
42 changes: 23 additions & 19 deletions sdk/src/manifest_store_report.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,25 +229,29 @@ impl ManifestStoreReport {
// is this an ingredient
if let Some(ref c2pa_manifest) = &ingredient_assertion.c2pa_manifest {
let label = Store::manifest_label_from_path(&c2pa_manifest.url());
let hash = &c2pa_manifest.hash()[..5];

if let Some(ingredient_claim) = store.get_claim(&label) {
// create new node
let data = if name_only {
format!("{}_{}", ingredient_assertion.title, Hexlify(hash))
} else {
format!("Asset:{}, Manifest:{}", ingredient_assertion.title, label)
};

let new_token = current_token.append(tree, data);

ManifestStoreReport::populate_node(
tree,
store,
ingredient_claim,
&new_token,
name_only,
)?;
if let Some(hash) = c2pa_manifest.hash().get(0..5) {
if let Some(ingredient_claim) = store.get_claim(&label) {
// create new node
let data = if name_only {
format!("{}_{}", ingredient_assertion.title, Hexlify(hash))
} else {
format!("Asset:{}, Manifest:{}", ingredient_assertion.title, label)
};

let new_token = current_token.append(tree, data);

ManifestStoreReport::populate_node(
tree,
store,
ingredient_claim,
&new_token,
name_only,
)?;
}
} else {
return Err(crate::Error::InvalidAsset(
"Manifest hash too short".to_string(),
));
}
} else {
let asset_name = &ingredient_assertion.title;
Expand Down

0 comments on commit 1bd44af

Please sign in to comment.