Skip to content

Commit

Permalink
Merge #1360: [backport] consensus decode from finite decoder.
Browse files Browse the repository at this point in the history
a0489d4 fuzz: use travis-fuzz.sh in CI (Andrew Poelstra)
4c6f9b3 fuzz: remove mysteriously-not-necessary quotes from gh action script (Andrew Poelstra)
7baa21c fuzz: disable features in honggfuzz (Andrew Poelstra)
e003e57 Add `consensus_decode_from_finite_reader` optimization (Dawid Ciężarkiewicz)

Pull request description:

  Backport for #1023. Required for #997. Addresses issues like #1359

ACKs for top commit:
  tcharding:
    ACK a0489d4
  TheBlueMatt:
    ACK a0489d4.

Tree-SHA512: 9145d9666e35ae77598aaecf89222c7234637b57ded39b69fbb93ee9ce01c6d7c938b36a2d86319ba84155f2424e524386593d6c0d7af449be6bd118f729fd64
  • Loading branch information
TheBlueMatt committed Nov 2, 2022
2 parents 219aa59 + a0489d4 commit bd59925
Show file tree
Hide file tree
Showing 9 changed files with 231 additions and 97 deletions.
7 changes: 1 addition & 6 deletions .github/workflows/fuzz.yml
Expand Up @@ -7,8 +7,6 @@ jobs:
fuzz:
if: ${{ !github.event.act }}
runs-on: ubuntu-20.04
env:
HFUZZ_BUILD_ARGS: "--features honggfuzz_fuzz"
strategy:
fail-fast: false
matrix:
Expand All @@ -30,11 +28,8 @@ jobs:
toolchain: 1.58
override: true
profile: minimal
- run: cargo install honggfuzz
if: steps.cache-fuzz.outputs.cache-hit != 'true'
- run: echo "HFUZZ_RUN_ARGS=\"--run_time 30 --exit_upon_crash -v -f hfuzz_input/${{ matrix.fuzz_target }}/input\"" >> $GITHUB_ENV
- name: fuzz
run: cd fuzz && cargo hfuzz run ${{ matrix.fuzz_target }}
run: cd fuzz && ./travis-fuzz.sh "${{ matrix.fuzz_target }}"
- run: echo "${{ matrix.fuzz_target }}.rs" >executed_${{ matrix.fuzz_target }}
- uses: actions/upload-artifact@v2
with:
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "bitcoin"
version = "0.28.1"
version = "0.28.2"
authors = ["Andrew Poelstra <apoelstra@wpsoftware.net>"]
license = "CC0-1.0"
homepage = "https://github.com/rust-bitcoin/rust-bitcoin/"
Expand Down
2 changes: 1 addition & 1 deletion fuzz/Cargo.toml
Expand Up @@ -12,7 +12,7 @@ afl_fuzz = ["afl"]
honggfuzz_fuzz = ["honggfuzz"]

[dependencies]
honggfuzz = { version = "0.5", optional = true }
honggfuzz = { version = "0.5", optional = true, default-features = false }
afl = { version = "0.4", optional = true }
bitcoin = { path = ".." }

Expand Down
14 changes: 12 additions & 2 deletions fuzz/travis-fuzz.sh
Expand Up @@ -8,9 +8,19 @@ if [ ${incorrectFilenames} -gt 0 ]; then
exit 2
fi

if [ "$1" == "" ]; then
TARGETS=fuzz_targets/*
else
TARGETS=fuzz_targets/"$1".rs
fi

cargo --version
rustc --version

# Testing
cargo install --force honggfuzz
for TARGET in fuzz_targets/*; do
cargo install --force honggfuzz --no-default-features
for TARGET in $TARGETS; do
echo "Fuzzing target $TARGET"
FILENAME=$(basename $TARGET)
FILE="${FILENAME%.*}"
if [ -d hfuzz_input/$FILE ]; then
Expand Down
8 changes: 7 additions & 1 deletion src/blockdata/script.rs
Expand Up @@ -23,6 +23,7 @@
//! This module provides the structures and functions needed to support scripts.
//!

use consensus::encode::MAX_VEC_SIZE;
use prelude::*;

use io;
Expand Down Expand Up @@ -1067,9 +1068,14 @@ impl Encodable for Script {
}

impl Decodable for Script {
#[inline]
fn consensus_decode_from_finite_reader<D: io::Read>(d: D) -> Result<Self, encode::Error> {
Ok(Script(Decodable::consensus_decode_from_finite_reader(d)?))
}

#[inline]
fn consensus_decode<D: io::Read>(d: D) -> Result<Self, encode::Error> {
Ok(Script(Decodable::consensus_decode(d)?))
Self::consensus_decode_from_finite_reader(d.take(MAX_VEC_SIZE as u64))
}
}

Expand Down
39 changes: 24 additions & 15 deletions src/blockdata/transaction.rs
Expand Up @@ -638,14 +638,20 @@ impl Encodable for TxIn {
}
}
impl Decodable for TxIn {
fn consensus_decode<D: io::Read>(mut d: D) -> Result<Self, encode::Error> {
#[inline]
fn consensus_decode_from_finite_reader<D: io::Read>(mut d: D) -> Result<Self, encode::Error> {
Ok(TxIn {
previous_output: Decodable::consensus_decode(&mut d)?,
script_sig: Decodable::consensus_decode(&mut d)?,
sequence: Decodable::consensus_decode(d)?,
previous_output: Decodable::consensus_decode_from_finite_reader(&mut d)?,
script_sig: Decodable::consensus_decode_from_finite_reader(&mut d)?,
sequence: Decodable::consensus_decode_from_finite_reader(d)?,
witness: Witness::default(),
})
}

#[inline]
fn consensus_decode<D: io::Read>(d: D) -> Result<Self, encode::Error> {
Self::consensus_decode_from_finite_reader(d.take(MAX_VEC_SIZE as u64))
}
}

impl Encodable for Transaction {
Expand Down Expand Up @@ -679,20 +685,19 @@ impl Encodable for Transaction {
}

impl Decodable for Transaction {
fn consensus_decode<D: io::Read>(d: D) -> Result<Self, encode::Error> {
let mut d = d.take(MAX_VEC_SIZE as u64);
let version = i32::consensus_decode(&mut d)?;
let input = Vec::<TxIn>::consensus_decode(&mut d)?;
fn consensus_decode_from_finite_reader<D: io::Read>(mut d: D) -> Result<Self, encode::Error> {
let version = i32::consensus_decode_from_finite_reader(&mut d)?;
let input = Vec::<TxIn>::consensus_decode_from_finite_reader(&mut d)?;
// segwit
if input.is_empty() {
let segwit_flag = u8::consensus_decode(&mut d)?;
let segwit_flag = u8::consensus_decode_from_finite_reader(&mut d)?;
match segwit_flag {
// BIP144 input witnesses
1 => {
let mut input = Vec::<TxIn>::consensus_decode(&mut d)?;
let output = Vec::<TxOut>::consensus_decode(&mut d)?;
let mut input = Vec::<TxIn>::consensus_decode_from_finite_reader(&mut d)?;
let output = Vec::<TxOut>::consensus_decode_from_finite_reader(&mut d)?;
for txin in input.iter_mut() {
txin.witness = Decodable::consensus_decode(&mut d)?;
txin.witness = Decodable::consensus_decode_from_finite_reader(&mut d)?;
}
if !input.is_empty() && input.iter().all(|input| input.witness.is_empty()) {
Err(encode::Error::ParseFailed("witness flag set but no witnesses present"))
Expand All @@ -701,7 +706,7 @@ impl Decodable for Transaction {
version,
input,
output,
lock_time: Decodable::consensus_decode(d)?,
lock_time: Decodable::consensus_decode_from_finite_reader(d)?,
})
}
}
Expand All @@ -713,11 +718,15 @@ impl Decodable for Transaction {
Ok(Transaction {
version,
input,
output: Decodable::consensus_decode(&mut d)?,
lock_time: Decodable::consensus_decode(d)?,
output: Decodable::consensus_decode_from_finite_reader(&mut d)?,
lock_time: Decodable::consensus_decode_from_finite_reader(d)?,
})
}
}

fn consensus_decode<D: io::Read>(d: D) -> Result<Self, encode::Error> {
Self::consensus_decode_from_finite_reader(d.take(MAX_VEC_SIZE as u64))
}
}

/// This type is consensus valid but an input including it would prevent the transaction from
Expand Down

0 comments on commit bd59925

Please sign in to comment.