Fix SusPay condition check in Finish:

On a SusPayFinish, a check is added to make sure that the
presented digest matches the digest in the SusPay ledger
entry. Another check is added to make Finish transactions
containing sfProof fields that are not 32 bytes malformed.

This includes regression unit tests.
This commit is contained in:
Vinnie Falco
2015-08-28 12:14:04 -07:00
parent 0f05ebd834
commit f32be2b28d
2 changed files with 35 additions and 2 deletions

View File

@@ -106,11 +106,12 @@ struct SusPay_test : public beast::unit_test::suite
return jv; return jv;
} }
template <class Proof>
static static
Json::Value Json::Value
finish (jtx::Account const& account, finish (jtx::Account const& account,
jtx::Account const& from, std::uint32_t seq, jtx::Account const& from, std::uint32_t seq,
uint256 const& digest, uint256 const& preimage) uint256 const& digest, Proof const& proof)
{ {
Json::Value jv; Json::Value jv;
jv[jss::TransactionType] = "SuspendedPaymentFinish"; jv[jss::TransactionType] = "SuspendedPaymentFinish";
@@ -120,7 +121,7 @@ struct SusPay_test : public beast::unit_test::suite
jv["OfferSequence"] = seq; jv["OfferSequence"] = seq;
jv["Method"] = 1; jv["Method"] = 1;
jv["Digest"] = to_string(digest); jv["Digest"] = to_string(digest);
jv["Proof"] = to_string(preimage); jv["Proof"] = to_string(proof);
return jv; return jv;
} }
@@ -304,6 +305,31 @@ struct SusPay_test : public beast::unit_test::suite
expect((*env.le("alice"))[sfOwnerCount] == 1); expect((*env.le("alice"))[sfOwnerCount] == 1);
env.require(balance("carol", XRP(5000))); env.require(balance("carol", XRP(5000)));
} }
{
Env env(*this);
auto T = [&env](NetClock::duration const& d)
{ return env.clock.now() + d; };
env.fund(XRP(5000), "alice", "bob", "carol");
auto const c = cond("receipt");
auto const seq = env.seq("alice");
env(condpay("alice", "carol", XRP(1000), c.first, T(S{1})));
// wrong digest
auto const cx = cond("bad");
env(finish("bob", "alice", seq, cx.first, cx.second), ter(tecNO_PERMISSION));
}
{
Env env(*this);
auto T = [&env](NetClock::duration const& d)
{ return env.clock.now() + d; };
env.fund(XRP(5000), "alice", "bob", "carol");
auto const p = from_hex_text<uint128>(
"0102030405060708090A0B0C0D0E0F");
auto const d = digest(p);
auto const seq = env.seq("alice");
env(condpay("alice", "carol", XRP(1000), d, T(S{1})));
// bad digest size
env(finish("bob", "alice", seq, d, p), ter(temMALFORMED));
}
} }
void void

View File

@@ -272,6 +272,8 @@ SusPayFinish::preflight (PreflightContext const& ctx)
return temMALFORMED; return temMALFORMED;
if (! ctx.tx[~sfProof]) if (! ctx.tx[~sfProof])
return temMALFORMED; return temMALFORMED;
if (ctx.tx[~sfProof]->size() != 32)
return temMALFORMED;
sha256_hasher h; sha256_hasher h;
using beast::hash_append; using beast::hash_append;
hash_append(h, ctx.tx[sfProof]); hash_append(h, ctx.tx[sfProof]);
@@ -324,6 +326,11 @@ SusPayFinish::doApply()
ctx_.view().info().parentCloseTime) ctx_.view().info().parentCloseTime)
return tecNO_PERMISSION; return tecNO_PERMISSION;
// Same digest?
if ((*slep)[~sfDigest] && (! ctx_.tx[~sfMethod] ||
(ctx_.tx[~sfDigest] != (*slep)[~sfDigest])))
return tecNO_PERMISSION;
AccountID const account = (*slep)[sfAccount]; AccountID const account = (*slep)[sfAccount];
// Remove SusPay from owner directory // Remove SusPay from owner directory