axone-protocol / axoned

⛓️ Axone blockchain 💫
https://axone.xyz
Apache License 2.0
164 stars 128 forks source link

🛡️ Halt Operation can lead to DOS #624

Closed ccamel closed 4 months ago

ccamel commented 6 months ago

[!NOTE] Severity: Info target: v7.1.0 - Commit: 3c854270b006db30aa3894da2cdba10cc31b8c5f Ref: OKP4 Blockchain Audit Report v1.0 - 02-05-2024 - BlockApex

Description

Okp4 supports rich set of prolog predicates. These predicates allows to define complex business logic and agreements conditions which can be queried and evaluated on-chain. Okp4 provides options to whitelist and blacklist the set of prolog predicates. One such predicates which caught our specific attention during our audit was halt/1. As per the swi-prolog documentation.

Halt: Terminate Prolog execution with default exit code using halt/1. The default exit code is normally 0, but can be 1 if one of the Prolog flags on_error or on_warning is set to status and there have been errors or warnings.

If the Blockchain is bootstrapped using the default configuration and 'halt' is specifically not blacklisted opens the room for potential usage of the predicate which would result is the termination of the underlaying node.

Recommandation

If the halt/1 is executed it would result in the termination of the blockchain process.

Although Okp4 explicitly mentions the potential impact of the halt/1 butwe find it necessary to mention that since okp4 recommend it to blacklist this predicate, we propose that this predicate should be removed from the code because the potential impact of someone not blacklisting is very critical as it would result in the terminating of the Node.

ccamel commented 5 months ago

I agree with the recommandation to exclude potentially dangerous built-in predicates provided by ichiban/prolog (like halt/1) from the registry of available predicates. Although the whitelist/blacklist exclusion mechanism offers some control, it is not absolute and can be overridden through governance. Furthermore, given the potential impact of these predicates, there is no justification for their existence in the protocol.

I'm therefore in favour of the following strategic approach: only declare native predicates from the logic module in the registry. This allows:

The predicates from ichiban/prolog that we want to integrate as they are into the registry must therefore be re-exported in the logic module and properly documented. This is already our practice, as demonstrated with the consult/1 predicate:


// x/logic/predicate/file.go
// ... proper documentation ...
func Consult(vm *engine.VM, file engine.Term, cont engine.Cont, env *engine.Env) *engine.Promise {
    return engine.Consult(vm, file, cont, env)
}