From caa486f19c84e24e50d8d4b30c0484a1b3bd387a Mon Sep 17 00:00:00 2001 From: tequ Date: Thu, 16 Oct 2025 15:34:30 +0900 Subject: [PATCH] additional defensive check, malformed check, --- src/ripple/app/tx/impl/SetCron.cpp | 59 +++++++++++++++--------------- src/test/app/Cron_test.cpp | 15 ++++++-- 2 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/ripple/app/tx/impl/SetCron.cpp b/src/ripple/app/tx/impl/SetCron.cpp index e487ee2e1..af5317ee0 100644 --- a/src/ripple/app/tx/impl/SetCron.cpp +++ b/src/ripple/app/tx/impl/SetCron.cpp @@ -61,15 +61,25 @@ SetCron::preflight(PreflightContext const& ctx) bool const hasDelay = tx.isFieldPresent(sfDelaySeconds); bool const hasRepeat = tx.isFieldPresent(sfRepeatCount); - if (hasRepeat && !hasDelay) + if (tx.isFlag(tfCronUnset)) { - JLOG(j.warn()) << "SetCron: DelaySeconds must also be specified when " - "RepeatCount is present."; - return temMALFORMED; + if (hasDelay || hasRepeat) + { + JLOG(j.debug()) << "SetCron: tfCronUnset flag cannot be used with " + "DelaySeconds or RepeatCount."; + return temMALFORMED; + } } - - if (hasDelay) + else { + if (!hasDelay) + { + JLOG(j.debug()) << "SetCron: DelaySeconds must be " + "specified to create a cron."; + return temMALFORMED; + } + + // check delay is not too high auto delay = tx.getFieldU32(sfDelaySeconds); if (delay > 31536000UL /* 365 days in seconds */) { @@ -77,27 +87,18 @@ SetCron::preflight(PreflightContext const& ctx) "days in seconds)."; return temMALFORMED; } - } - if (hasRepeat) - { - auto recur = tx.getFieldU32(sfRepeatCount); - if (recur > 256) + // check repeat is not too high + if (hasRepeat) { - JLOG(j.debug()) - << "SetCron: RepeatCount too high. Limit is 256. Issue " - "new SetCron to increase."; - return temMALFORMED; - } - } - - if (tx.isFlag(tfCronUnset)) - { - if (hasDelay || hasRepeat) - { - JLOG(j.warn()) << "SetCron: tfCronUnset flag cannot be used with " - "DelaySeconds or RepeatCount."; - return temMALFORMED; + auto recur = tx.getFieldU32(sfRepeatCount); + if (recur > 256) + { + JLOG(j.debug()) + << "SetCron: RepeatCount too high. Limit is 256. Issue " + "new SetCron to increase."; + return temMALFORMED; + } } } @@ -136,7 +137,8 @@ SetCron::doApply() if (!isDelete) { - delay = tx.getFieldU32(sfDelaySeconds); + if (tx.isFieldPresent(sfDelaySeconds)) + delay = tx.getFieldU32(sfDelaySeconds); if (tx.isFieldPresent(sfRepeatCount)) recur = tx.getFieldU32(sfRepeatCount); } @@ -242,9 +244,8 @@ SetCron::calculateBaseFee(ReadView const& view, STTx const& tx) auto const baseFee = Transactor::calculateBaseFee(view, tx); auto const hasRepeat = tx.isFieldPresent(sfRepeatCount); - auto const hasDelay = tx.isFieldPresent(sfDelaySeconds); - if (!hasRepeat && !hasDelay) + if (tx.isFlag(tfCronUnset)) // delete cron return baseFee; @@ -253,7 +254,7 @@ SetCron::calculateBaseFee(ReadView const& view, STTx const& tx) // single Cron txn (2). For a RepeatCount of 1 we have this txn, // the first time the cron executes, and the second time (3). uint32_t const additionalExpectedExecutions = - tx.getFieldU32(sfRepeatCount) + 1; + hasRepeat ? tx.getFieldU32(sfRepeatCount) + 1 : 1; auto const additionalFee = baseFee * additionalExpectedExecutions; if (baseFee + additionalFee < baseFee) diff --git a/src/test/app/Cron_test.cpp b/src/test/app/Cron_test.cpp index c6d1d075e..821096ff4 100644 --- a/src/test/app/Cron_test.cpp +++ b/src/test/app/Cron_test.cpp @@ -50,7 +50,7 @@ struct Cron_test : public beast::unit_test::suite auto tx = cron::set(alice); // CLAIM - env(cron::set(alice), fee(XRP(1)), expectResult); + env(cron::set(alice), cron::delay(100), fee(XRP(1)), expectResult); env.close(); } } @@ -102,10 +102,16 @@ struct Cron_test : public beast::unit_test::suite // delete expected = baseFee; - env(cron::set(alice), fee(expected - 1), ter(telINSUF_FEE_P)); + env(cron::set(alice), + txflags(tfCronUnset), + fee(expected - 1), + ter(telINSUF_FEE_P)); env.close(); - env(cron::set(alice), fee(expected), ter(tesSUCCESS)); + env(cron::set(alice), + txflags(tfCronUnset), + fee(expected), + ter(tesSUCCESS)); env.close(); } @@ -137,6 +143,9 @@ struct Cron_test : public beast::unit_test::suite // temMALFORMED { + // Invalid both DelaySeconds and RepeatCount are not specified + env(cron::set(alice), ter(temMALFORMED)); + // Invalid DelaySeconds and RepeatCount combination // (only RepeatCount specified) env(cron::set(alice), cron::repeat(256), ter(temMALFORMED));