Skip to content

Commit

Permalink
additional fixes for #8 XRPLF#10 and XRPLF#14
Browse files Browse the repository at this point in the history
  • Loading branch information
@RichardAH
RichardAH committed Feb 17, 2023
1 parent 7a008c2 commit 5f5a947
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 70 deletions.
4 changes: 2 additions & 2 deletions src/ripple/app/hook/Macro.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@

// ptr = pointer inside the wasm memory space
#define NOT_IN_BOUNDS(ptr, len, memory_length)\
((static_cast<uint64_t>(ptr) > static_cast<uint64_t>(memory_length)) || \
((static_cast<uint64_t>(ptr) + static_cast<uint64_t>(len)) >= static_cast<uint64_t>(memory_length)))
((static_cast<uint64_t>(ptr) >= static_cast<uint64_t>(memory_length)) || \
((static_cast<uint64_t>(ptr) + static_cast<uint64_t>(len)) > static_cast<uint64_t>(memory_length)))

#define HOOK_EXIT(read_ptr, read_len, error_code, exit_type)\
{\
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/hook/applyHook.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ namespace hook
bool /* retval of true means an error */
gatherHookParameters(
std::shared_ptr<ripple::STLedgerEntry> const& hookDef,
ripple::STObject const* hookObj,
ripple::STObject const& hookObj,
std::map<std::vector<uint8_t>, std::vector<uint8_t>>& parameters,
beast::Journal const& j_);

Expand Down
58 changes: 26 additions & 32 deletions src/ripple/app/hook/impl/applyHook.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,16 +632,16 @@ get_free_slot(hook::HookContext& hookCtx)
// slot ahead of when the counter gets there
do
{
if (hookCtx.slot_counter >= hook_api::max_slots)
return {};

slot_into = ++hookCtx.slot_counter;
}
while (hookCtx.slot.find(slot_into) != hookCtx.slot.end() &&
// this condition should always be met, if for some reason, somehow it is not
// then we will return the final slot every time.
hookCtx.slot_counter <= hook_api::max_slots);

if (hookCtx.slot_counter > hook_api::max_slots)
return {};

return slot_into;
}

Expand Down Expand Up @@ -1529,46 +1529,43 @@ DEFINE_HOOK_FUNCTION(

// we do this by iterating the hooks installed on the foreign account and in turn their grants and namespaces
auto const& hooks = sle->getFieldArray(sfHooks);
for (auto const& hook : hooks)
for (auto const& hookObj : hooks)
{
STObject const* hookObj = dynamic_cast<STObject const*>(&hook);

// skip blank entries
if (!hookObj->isFieldPresent(sfHookHash))
if (!hookObj.isFieldPresent(sfHookHash))
continue;

if (!hookObj->isFieldPresent(sfHookGrants))
if (!hookObj.isFieldPresent(sfHookGrants))
continue;

auto const& hookGrants = hookObj->getFieldArray(sfHookGrants);
auto const& hookGrants = hookObj.getFieldArray(sfHookGrants);

if (hookGrants.size() < 1)
continue;

// the grant allows the hook to modify the granter's namespace only
if (hookObj->isFieldPresent(sfHookNamespace))
if (hookObj.isFieldPresent(sfHookNamespace))
{
if (hookObj->getFieldH256(sfHookNamespace) != ns)
if (hookObj.getFieldH256(sfHookNamespace) != ns)
continue;
}
else
{
// fetch the hook definition
auto const def = view.read(ripple::keylet::hookDefinition(hookObj->getFieldH256(sfHookHash)));
auto const def = view.read(ripple::keylet::hookDefinition(hookObj.getFieldH256(sfHookHash)));
if (!def) // should never happen except in a rare race condition
continue;
if (def->getFieldH256(sfHookNamespace) != ns)
continue;
}

// this is expensive search so we'll disallow after one failed attempt
for (auto const& hookGrant : hookGrants)
for (auto const& hookGrantObj : hookGrants)
{
STObject const* hookGrantObj = dynamic_cast<STObject const*>(&hookGrant);
bool hasAuthorizedField = hookGrantObj->isFieldPresent(sfAuthorize);
bool hasAuthorizedField = hookGrantObj.isFieldPresent(sfAuthorize);

if (hookGrantObj->getFieldH256(sfHookHash) == hookCtx.result.hookHash &&
(!hasAuthorizedField || hookGrantObj->getAccountID(sfAuthorize) == hookCtx.result.account))
if (hookGrantObj.getFieldH256(sfHookHash) == hookCtx.result.hookHash &&
(!hasAuthorizedField || hookGrantObj.getAccountID(sfAuthorize) == hookCtx.result.account))
{
found_auth = true;
break;
Expand Down Expand Up @@ -1659,7 +1656,7 @@ bool /* retval of true means an error */
hook::
gatherHookParameters(
std::shared_ptr<ripple::STLedgerEntry> const& hookDef,
ripple::STObject const* hookObj,
ripple::STObject const& hookObj,
std::map<std::vector<uint8_t>, std::vector<uint8_t>>& parameters,
beast::Journal const& j_)
{
Expand All @@ -1672,22 +1669,20 @@ gatherHookParameters(

// first defaults
auto const& defaultParameters = hookDef->getFieldArray(sfHookParameters);
for (auto const& hookParameter : defaultParameters)
for (auto const& hookParameterObj : defaultParameters)
{
auto const& hookParameterObj = dynamic_cast<STObject const*>(&hookParameter);
parameters[hookParameterObj->getFieldVL(sfHookParameterName)] =
hookParameterObj->getFieldVL(sfHookParameterValue);
parameters[hookParameterObj.getFieldVL(sfHookParameterName)] =
hookParameterObj.getFieldVL(sfHookParameterValue);
}

// and then custom
if (hookObj->isFieldPresent(sfHookParameters))
if (hookObj.isFieldPresent(sfHookParameters))
{
auto const& hookParameters = hookObj->getFieldArray(sfHookParameters);
for (auto const& hookParameter : hookParameters)
auto const& hookParameters = hookObj.getFieldArray(sfHookParameters);
for (auto const& hookParameterObj : hookParameters)
{
auto const& hookParameterObj = dynamic_cast<STObject const*>(&hookParameter);
parameters[hookParameterObj->getFieldVL(sfHookParameterName)] =
hookParameterObj->getFieldVL(sfHookParameterValue);
parameters[hookParameterObj.getFieldVL(sfHookParameterName)] =
hookParameterObj.getFieldVL(sfHookParameterValue);
}
}
return false;
Expand Down Expand Up @@ -5254,12 +5249,11 @@ DEFINE_HOOK_FUNCTION(

ripple::STArray const& hooks = hookSLE->getFieldArray(sfHooks);
bool found = false;
for (auto const& hook : hooks)
for (auto const& hookObj : hooks)
{
auto const& hookObj = dynamic_cast<STObject const*>(&hook);
if (hookObj->isFieldPresent(sfHookHash))
if (hookObj.isFieldPresent(sfHookHash))
{
if (hookObj->getFieldH256(sfHookHash) == hash)
if (hookObj.getFieldH256(sfHookHash) == hash)
{
found = true;
break;
Expand Down
62 changes: 27 additions & 35 deletions src/ripple/app/tx/impl/Transactor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,12 @@ calculateHookChainFee(

auto const& hooks = hookSLE->getFieldArray(sfHooks);

for (auto const& hook : hooks)
for (auto const& hookObj : hooks)
{
ripple::STObject const* hookObj = dynamic_cast<ripple::STObject const*>(&hook);

if (!hookObj->isFieldPresent(sfHookHash)) // skip blanks
if (!hookObj.isFieldPresent(sfHookHash)) // skip blanks
continue;

uint256 const& hash = hookObj->getFieldH256(sfHookHash);
uint256 const& hash = hookObj.getFieldH256(sfHookHash);

std::shared_ptr<SLE const> hookDef = view.read(keylet::hookDefinition(hash));

Expand All @@ -236,13 +234,13 @@ calculateHookChainFee(
}

// check if the hook can fire
uint256 hookOn = (hookObj->isFieldPresent(sfHookOn)
? hookObj->getFieldH256(sfHookOn)
uint256 hookOn = (hookObj.isFieldPresent(sfHookOn)
? hookObj.getFieldH256(sfHookOn)
: hookDef->getFieldH256(sfHookOn));

uint32_t flags = 0;
if (hookObj->isFieldPresent(sfFlags))
flags = hookObj->getFieldU32(sfFlags);
if (hookObj.isFieldPresent(sfFlags))
flags = hookObj.getFieldU32(sfFlags);
else
flags = hookDef->getFieldU32(sfFlags);

Expand Down Expand Up @@ -997,17 +995,15 @@ executeHookChain(
auto const& hooks = hookSLE->getFieldArray(sfHooks);
uint8_t hook_no = 0;

for (auto const& hook : hooks)
for (auto const& hookObj : hooks)
{
hook_no++;

ripple::STObject const* hookObj = dynamic_cast<ripple::STObject const*>(&hook);

if (!hookObj->isFieldPresent(sfHookHash)) // skip blanks
if (!hookObj.isFieldPresent(sfHookHash)) // skip blanks
continue;

// lookup hook definition
uint256 const& hookHash = hookObj->getFieldH256(sfHookHash);
uint256 const& hookHash = hookObj.getFieldH256(sfHookHash);

if (hookSkips.find(hookHash) != hookSkips.end())
{
Expand All @@ -1025,15 +1021,15 @@ executeHookChain(
}

// check if the hook can fire
uint256 hookOn = (hookObj->isFieldPresent(sfHookOn)
? hookObj->getFieldH256(sfHookOn)
uint256 hookOn = (hookObj.isFieldPresent(sfHookOn)
? hookObj.getFieldH256(sfHookOn)
: hookDef->getFieldH256(sfHookOn));

if (!hook::canHook(ctx_.tx.getTxnType(), hookOn))
continue; // skip if it can't

uint32_t flags = (hookObj->isFieldPresent(sfFlags) ?
hookObj->getFieldU32(sfFlags) : hookDef->getFieldU32(sfFlags));
uint32_t flags = (hookObj.isFieldPresent(sfFlags) ?
hookObj.getFieldU32(sfFlags) : hookDef->getFieldU32(sfFlags));

JLOG(j_.trace())
<< "HookChainExecution: " << hookHash
Expand All @@ -1045,8 +1041,8 @@ executeHookChain(

// fetch the namespace either from the hook object of, if absent, the hook def
uint256 const& ns =
(hookObj->isFieldPresent(sfHookNamespace)
? hookObj->getFieldH256(sfHookNamespace)
(hookObj.isFieldPresent(sfHookNamespace)
? hookObj.getFieldH256(sfHookNamespace)
: hookDef->getFieldH256(sfHookNamespace));

// gather parameters
Expand Down Expand Up @@ -1161,22 +1157,20 @@ Transactor::doHookCallback(std::shared_ptr<STObject const> const& provisionalMet
bool found = false;
auto const& hooks = hooksCallback->getFieldArray(sfHooks);
uint8_t hook_no = 0;
for (auto const& hook : hooks)
for (auto const& hookObj : hooks)
{
hook_no++;

STObject const* hookObj = dynamic_cast<STObject const*>(&hook);

if (!hookObj->isFieldPresent(sfHookHash)) // skip blanks
if (!hookObj.isFieldPresent(sfHookHash)) // skip blanks
continue;

if (hookObj->getFieldH256(sfHookHash) != callbackHookHash)
if (hookObj.getFieldH256(sfHookHash) != callbackHookHash)
continue;

// fetch the namespace either from the hook object of, if absent, the hook def
uint256 const& ns =
(hookObj->isFieldPresent(sfHookNamespace)
? hookObj->getFieldH256(sfHookNamespace)
(hookObj.isFieldPresent(sfHookNamespace)
? hookObj.getFieldH256(sfHookNamespace)
: hookDef->getFieldH256(sfHookNamespace));

executedHookCount_++;
Expand Down Expand Up @@ -1433,18 +1427,16 @@ Transactor::doAgainAsWeak(

auto const& hooks = hooksArray->getFieldArray(sfHooks);
uint8_t hook_no = 0;
for (auto const& hook : hooks)
for (auto const& hookObj : hooks)
{
hook_no++;

STObject const* hookObj = dynamic_cast<STObject const*>(&hook);

if (!hookObj->isFieldPresent(sfHookHash)) // skip blanks
if (!hookObj.isFieldPresent(sfHookHash)) // skip blanks
continue;

uint256 const& hookHash = hookObj->getFieldH256(sfHookHash);
uint256 const& hookHash = hookObj.getFieldH256(sfHookHash);

if (hookHashes.find(hookObj->getFieldH256(sfHookHash)) == hookHashes.end())
if (hookHashes.find(hookObj.getFieldH256(sfHookHash)) == hookHashes.end())
continue;

auto const& hookDef = view().peek(keylet::hookDefinition(hookHash));
Expand All @@ -1458,8 +1450,8 @@ Transactor::doAgainAsWeak(

// fetch the namespace either from the hook object of, if absent, the hook def
uint256 const& ns =
(hookObj->isFieldPresent(sfHookNamespace)
? hookObj->getFieldH256(sfHookNamespace)
(hookObj.isFieldPresent(sfHookNamespace)
? hookObj.getFieldH256(sfHookNamespace)
: hookDef->getFieldH256(sfHookNamespace));

executedHookCount_++;
Expand Down

0 comments on commit 5f5a947

Please sign in to comment.