Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: apply security audit results #1196

Merged
merged 2 commits into from
Jul 9, 2020
Merged

fix: apply security audit results #1196

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
603c431
fix: apply security audit results
cryptonemo Jul 1, 2020
276bfdf
fix: apply snark audit updates
cryptonemo Jul 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion fil-proofs-tooling/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ rayon = "1.3.0"
flexi_logger = "0.14.7"
typenum = "1.11.2"
generic-array = "0.13.2"
tempdir = "0.3.7"

[features]
default = ["gpu", "measurements"]
Expand Down
3 changes: 1 addition & 2 deletions fil-proofs-tooling/src/bin/update_tree_r_cache/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use generic_array::typenum::Unsigned;
use memmap::MmapOptions;
use merkletree::merkle::get_merkle_tree_len;
use merkletree::store::{ExternalReader, ReplicaConfig, Store, StoreConfig};
use tempdir::TempDir;

use filecoin_proofs::constants::*;
use filecoin_proofs::types::*;
Expand Down Expand Up @@ -244,7 +243,7 @@ fn run_verify(sector_size: usize, cache: PathBuf, replica_path: PathBuf) -> Resu

// Rebuild each of the tree_r_last base trees (in a new temp dir so as not to interfere
// with any existing ones on disk) and check if the roots match what's cached on disk
let tmp_dir = TempDir::new("tree-r-last-verify")?;
let tmp_dir = tempfile::tempdir().unwrap();
let tmp_path = tmp_dir.path();
create_dir_all(&tmp_path)?;

Expand Down
6 changes: 3 additions & 3 deletions storage-proofs/core/src/compound_proof.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,18 +202,18 @@ where
/// default trait methods.
fn circuit_proofs(
pub_in: &S::PublicInputs,
vanilla_proof: Vec<S::Proof>,
vanilla_proofs: Vec<S::Proof>,
pub_params: &S::PublicParams,
groth_params: &groth16::MappedParameters<Bls12>,
priority: bool,
) -> Result<Vec<groth16::Proof<Bls12>>> {
let mut rng = OsRng;
ensure!(
!vanilla_proof.is_empty(),
!vanilla_proofs.is_empty(),
"cannot create a circuit proof over missing vanilla proofs"
);

let circuits = vanilla_proof
let circuits = vanilla_proofs
.into_par_iter()
.enumerate()
.map(|(k, vanilla_proof)| {
Expand Down
1 change: 0 additions & 1 deletion storage-proofs/porep/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ hex = "0.4.2"
byteorder = "1.3.4"

[dev-dependencies]
tempdir = "0.3.7"
tempfile = "3"
rand_xorshift = "0.2.0"
criterion = "0.3.2"
Expand Down
4 changes: 4 additions & 0 deletions storage-proofs/porep/src/stacked/circuit/column.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ impl Column {
}

impl AllocatedColumn {
pub fn len(&self) -> usize {
self.rows.len()
}

/// Creates the column hash of this column.
pub fn hash<CS: ConstraintSystem<Bls12>>(
&self,
Expand Down
1 change: 1 addition & 0 deletions storage-proofs/porep/src/stacked/circuit/create_label.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ where
E: JubjubEngine,
CS: ConstraintSystem<E>,
{
assert!(replica_id.len() >= 32, "replica id is too small");
assert!(replica_id.len() <= 256, "replica id is too large");
assert_eq!(parents.len(), TOTAL_PARENTS, "invalid sized parents");

Expand Down
7 changes: 6 additions & 1 deletion storage-proofs/porep/src/stacked/circuit/params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ impl<Tree: MerkleTreeTrait, G: 'static + Hasher> Proof<Tree, G> {
..
} = self;

assert!(!drg_parents_proofs.is_empty());
assert!(!exp_parents_proofs.is_empty());

// -- verify initial data layer

// PrivateInput: data_leaf
Expand All @@ -112,11 +115,12 @@ impl<Tree: MerkleTreeTrait, G: 'static + Hasher> Proof<Tree, G> {
// -- verify replica column openings

// Private Inputs for the DRG parent nodes.
let mut drg_parents = Vec::new();
let mut drg_parents = Vec::with_capacity(layers);

for (i, parent) in drg_parents_proofs.into_iter().enumerate() {
let (parent_col, inclusion_path) =
parent.alloc(cs.namespace(|| format!("drg_parent_{}_num", i)))?;
assert_eq!(layers, parent_col.len());

// calculate column hash
let val = parent_col.hash(cs.namespace(|| format!("drg_parent_{}_constraint", i)))?;
Expand All @@ -136,6 +140,7 @@ impl<Tree: MerkleTreeTrait, G: 'static + Hasher> Proof<Tree, G> {
for (i, parent) in exp_parents_proofs.into_iter().enumerate() {
let (parent_col, inclusion_path) =
parent.alloc(cs.namespace(|| format!("exp_parent_{}_num", i)))?;
assert_eq!(layers, parent_col.len());

// calculate column hash
let val = parent_col.hash(cs.namespace(|| format!("exp_parent_{}_constraint", i)))?;
Expand Down
4 changes: 4 additions & 0 deletions storage-proofs/porep/src/stacked/vanilla/proof_scheme.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ impl<'a, 'c, Tree: 'static + MerkleTreeTrait, G: 'static + Hasher> ProofScheme<'
) -> bool {
let partition_challenges = public_params.layer_challenges.challenges_count_all();

assert_eq!(
partition_challenges.checked_mul(partitions),
Some(partition_challenges * partitions)
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, it would be okay if this overflows because the result would be conservative and, if anything, lead to requirements being judged unsatisfied when they 'should' be satisfied. Of course this cannot happen for correct protocol values anyway, and I agree a check is helpful for development.

That said, if this 'error' were hit in reality, it would probably mean we need to revise the code. Technically, values which would overflow could be 'correct'. None of this is a practical issue. I'm just thinking through the significance of the assertion.

I don't recommend or request it, but we could 'technically' return true if the assertion fails (even though this would undoubtedly be papering over a usage error).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I'll leave in the assert as-is, but realize they should not trigger in normal circumstances and the downside is that we're calculating values more than needed. If we return true if the check fails, there's probably no reason to do it since I think even logging it would likely be lost/ignored.

partition_challenges * partitions >= requirements.minimum_challenges
}
}
1 change: 0 additions & 1 deletion storage-proofs/post/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ neptune = { version = "1.0.1", features = ["gpu"] }
num_cpus = "1.10.1"

[dev-dependencies]
tempdir = "0.3.7"
tempfile = "3"
pretty_assertions = "0.6.1"
rand_xorshift = "0.2.0"
2 changes: 1 addition & 1 deletion storage-proofs/post/src/election/circuit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ mod tests {
let mut trees = BTreeMap::new();

// Construct and store an MT using a named store.
let temp_dir = tempdir::TempDir::new("tree").unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path();

for i in 0..5 {
Expand Down
2 changes: 1 addition & 1 deletion storage-proofs/post/src/election/compound.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ mod tests {
let mut trees = BTreeMap::new();

// Construct and store an MT using a named store.
let temp_dir = tempdir::TempDir::new("tree").unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path();

for i in 0..5 {
Expand Down
2 changes: 1 addition & 1 deletion storage-proofs/post/src/election/vanilla.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ mod tests {
let mut trees = BTreeMap::new();

// Construct and store an MT using a named store.
let temp_dir = tempdir::TempDir::new("tree").unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path();

for i in 0..5 {
Expand Down
2 changes: 1 addition & 1 deletion storage-proofs/post/src/fallback/circuit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ mod tests {
};

// Construct and store an MT using a named DiskStore.
let temp_dir = tempdir::TempDir::new("level_cache_tree_v1").unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path();

let mut pub_sectors = Vec::new();
Expand Down
2 changes: 1 addition & 1 deletion storage-proofs/post/src/fallback/compound.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ mod tests {
};

// Construct and store an MT using a named DiskStore.
let temp_dir = tempdir::TempDir::new("level_cache_tree_v1").unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path();

let mut pub_sectors = Vec::new();
Expand Down
16 changes: 13 additions & 3 deletions storage-proofs/post/src/fallback/vanilla.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,18 @@ impl<'a, Tree: 'a + MerkleTreeTrait> ProofScheme<'a> for FallbackPoSt<'a, Tree>
requirements: &Self::Requirements,
partitions: usize,
) -> bool {
partitions * public_params.sector_count * public_params.challenge_count
>= requirements.minimum_challenge_count
let checked = partitions * public_params.sector_count;

assert_eq!(
partitions.checked_mul(public_params.sector_count),
Some(checked)
);
assert_eq!(
checked.checked_mul(public_params.challenge_count),
Some(checked * public_params.challenge_count)
);

checked * public_params.challenge_count >= requirements.minimum_challenge_count
}
}

Expand Down Expand Up @@ -497,7 +507,7 @@ mod tests {
let prover_id = <Tree::Hasher as Hasher>::Domain::random(rng);

// Construct and store an MT using a named DiskStore.
let temp_dir = tempdir::TempDir::new("level_cache_tree").unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path();

let mut pub_sectors = Vec::new();
Expand Down
2 changes: 1 addition & 1 deletion storage-proofs/post/src/rational/circuit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ mod tests {
};

// Construct and store an MT using a named DiskStore.
let temp_dir = tempdir::TempDir::new("tree").unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path();

let (_data1, tree1) = generate_tree::<Tree, _>(rng, leaves, Some(temp_path.to_path_buf()));
Expand Down
2 changes: 1 addition & 1 deletion storage-proofs/post/src/rational/compound.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ mod tests {
let pub_params = RationalPoStCompound::<Tree>::setup(&setup_params).expect("setup failed");

// Construct and store an MT using a named DiskStore.
let temp_dir = tempdir::TempDir::new("tree").unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path();

let (_data1, tree1) = generate_tree::<Tree, _>(rng, leaves, Some(temp_path.to_path_buf()));
Expand Down
4 changes: 2 additions & 2 deletions storage-proofs/post/src/rational/vanilla.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ mod tests {
};

// Construct and store an MT using a named store.
let temp_dir = tempdir::TempDir::new("tree").unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path();

let (_data1, tree1) = generate_tree::<Tree, _>(rng, leaves, Some(temp_path.to_path_buf()));
Expand Down Expand Up @@ -438,7 +438,7 @@ mod tests {
};

// Construct and store an MT using a named store.
let temp_dir = tempdir::TempDir::new("tree").unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path();

let (_data, tree) = generate_tree::<Tree, _>(rng, leaves, Some(temp_path.to_path_buf()));
Expand Down