hats-finance / Intuition-0x538dbadc50cc87b281cd655f1edbc6ebda02a66a

The smart contracts of the Intuition protocol v1.
https://intuition.systems
Other
0 stars 1 forks source link

`_createTriple()` logic do not follow the intended design mentioned in documentation #55

Open hats-bug-reporter[bot] opened 3 months ago

hats-bug-reporter[bot] commented 3 months ago

Github username: @itsabinashb Twitter username: akatabletos Submission hash (on-chain): 0x3219a50d45531bb4608fd533f3fb64d9b794f62810d962c576e72e28c7afd21a Severity: medium

Description: Description\ The current logic of _createTriple() does not match with intended design mentioned in documentation, in case of adding Triples as Atom in Tripple.

Attack Scenario\

See the PoC section. Attachments

  1. Proof of Concept (PoC) File\ In documention it was mentioned how a Triple should be treated:

Intuition’s atomic unit of knowledge. [Atoms] can be used to represent [Subjects], [Predicates], [Objects], and [Triples]. All [Triples] are a composition of [Atoms], and [Triples] can be used as [Atoms] in other [Triples].

See the last sentense: [Triples] can be used as [Atoms] in other [Triples], but if you see the code in _createTriple() :

            // make sure that each id is not a triple vault id
            if (isTripleId(tripleAtomIds[i])) {
                revert Errors.MultiVault_VaultIsTriple(tripleAtomIds[i]);
            }

it is reverting when the one of 3 ids are of Triple. So, as result the Triple cannot be used as Atom while it should be.\

  1. Revised Code File (Optional)
itsabinashb commented 3 months ago

here is poc:

function test_cannotUseTripleAsAtomInTriple() external {
        vm.startPrank(alice, alice);

        // test values
        uint256 testAtomCost = getAtomCost();
        uint256 testDepositAmountTriple = 0.01 ether;

        // execute interaction - create atoms
        uint256 subjectId = ethMultiVault.createAtom{value: testAtomCost}("subject");
        uint256 predicateId = ethMultiVault.createAtom{value: testAtomCost}("predicate");
        uint256 objectId = ethMultiVault.createAtom{value: testAtomCost}("object");

        // execute interaction - create a triple
        uint256 id = ethMultiVault.createTriple{value: testDepositAmountTriple}(subjectId, predicateId, objectId);

        //@audit preparing for creating another Triple
        uint256 subjectId2 = ethMultiVault.createAtom{value: testAtomCost}("subjectSecond");
        uint256 predicateId2 = ethMultiVault.createAtom{value: testAtomCost}("predicateSecond");

        // @audit reverted
        vm.expectRevert(abi.encodeWithSelector(Errors.MultiVault_VaultIsTriple.selector,id));
        ethMultiVault.createTriple{value: testDepositAmountTriple}(subjectId2, predicateId2, id);  // @audit trying to use Triple as Atom

        vm.stopPrank();
    }

Run this in DepositTriple.t.sol file.

mihailo-maksa commented 3 months ago

The reported issue concerning the logic of _createTriple() not aligning with the documentation has been reviewed. Here is our detailed perspective:

Documentation and Design: The documentation correctly states that triples can be used as atoms in other triples. However, the current implementation of _createTriple() reverts if one of the IDs is a triple vault ID. This design decision was made to ensure clarity and prevent potential misuse or confusion within the protocol.

Intended Usage: To use a triple as an atom, users should create a new atom with the triple’s claim as the atomUri. This approach maintains the integrity of the protocol and ensures that triples are correctly represented as atoms when needed.

Severity Assessment: Since this issue does not introduce any vulnerabilities or risks to users and is primarily about aligning implementation with documentation, it is classified as a low severity enhancement. The current implementation ensures that triples are treated distinctly unless explicitly represented as atoms.

Conclusion: The documentation can be clarified to better explain the process of using triples as atoms by creating new atoms with the triple’s claim as the atomUri. This issue is more of a documentation enhancement rather than a security vulnerability.

Status: This issue is an enhancement.

Extra Considerations:

Comment for the Reporter: Thank you for highlighting this potential issue. The documentation correctly states that triples can be used as atoms in other triples. However, to achieve this, users should create a new atom with the triple’s claim as the atomUri. This makes the report and the PoC invalid as a security vulnerability. At best, it can be considered a suggestion to clarify our documentation, and as such will be treated as an enhancement.

itsabinashb commented 3 months ago

Thanks for your reply, as the issue highlights the misinformation/incomplete information provided by the documentation I think this should be considered as low severity. @mihailo-maksa

mihailo-maksa commented 2 months ago

We've decided to label this issue as minor out of goodwill. Thanks for your understanding.