Info

Download ZIP (49.1 KB)

Testing and Issues

You can test this entry and submit issues during the testing period of the Blockchain Contest contest.

Entries with serious issues will not be able to win the contest, but even minor issues might be important for overall results.

Voting

55

Comments

In my submission you may find multisig wallet, manual dns resolver and automatic dns resolver. All files for each task are in the correspondent folder. Each folder has the following structure:

./src - all sources files of the solution
./dist - compiled scripts that may be used for using smart contract (create the contract itself and messages to it)
./lib - folder for stdlib.fc
./test - source files for tests. May be used to understand better how each contract works
./temp - folder where all the tests are built and run.
build.sh - bash file that compiles func code and creates ready to use fift scripts in ./dist folder. Also builds and runs tests.
README.md - description of each contract - available scripts, get-methods, data structures used for messages and returned from get methods.
You have not added any comments yet...
by rating

Issues

Magic Python Nov 4, 2019 at 10:20
Multisig wallet:
plus:
Some tests are implemented.
It is possible to add a signature to order without modifying seqno.

minus:
Sadly it didn't work on our test (n=16, k=10). It is because of too late accept_message.
garbage_collect time depends on seqno.
It looks like a line that is not uncommented, but an order may be executed several times.
Not all features are implemented (merge signatures)
10
Eager Boar Nov 6, 2019 at 18:36
>garbage_collect time depends on seqno.
It was made intentionally, GC is pretty expensive, so I didn’t want to run it on each transaction.
Clever Turkey Nov 4, 2019 at 19:01
Manual DNS:
- dnsresolve truncates string on the first terminator. This is not a desired behavior for DNS.
- There is no way to completely delete expired domains.
- Public key change is not protected with seqno, so if a key is changed back, then anyone can replay the change.
- accept_message is called before seqno is checked, so anyone can drain smart contract balance.
- Maximum domain length is limited by 32 bytes.
- All dictionary keys are 256-bit, so there could be high storage costs.
10
Eager Boar Nov 6, 2019 at 18:38
>- Public key change is not protected with seqno, so if a key is changed back, then anyone can replay the change.

I assumed that each key pair is used only once and if the owner changes the key for the new one he will never use the old pair again, so there will be no change back to it.
Clever Turkey Nov 4, 2019 at 19:02
Automatic DNS:
+ Ability to change prices after contract creation.
- dnsresolve truncates string on the first terminator. This is not a desired behavior for DNS.
- No seqno checks in recv_external, so anyone can repeat prices change.
- seqno is checked for internal messages, so it is really hard to automatically change two domains simultaneously.
- Price is not calculated recursively for child cells.
- No garbage collection for expired domains.
- Maximum domain length is limited by 32 bytes.
- There is no way to customize domain expiration time.
- It is not possible to replace non-owned expired domain.
10
Eager Boar Nov 6, 2019 at 18:41
>- It is not possible to replace non-owned expired domain.

But every time a new domain is registered, -2 category is populated from the sender address and as far as new domains creation is possible only via internal messages (not from messages from nowhere) there is no way for a domain to be non-owned.
As I see, accept_message() is called before test of at least one signatures
3
D
Deleted Account Oct 18, 2019 at 11:02
Besides accepting message before checking the signatures
Multisignature contracts has no protection against replay of messages which sign already signed messages ( where msg_seqno < seq_num ).
2
D
Deleted Account Oct 18, 2019 at 11:03
Multisig:
Very inefficient signature check: each time all keys are iterated to find one which indeed signs the message.
2
Very interesting point, indeed. However lack of checking of seqno at all may allow malicious replay attack if ever PK change chain results in a loop (for example if Alice transfers to Bob, Bob trasfers to Charlie, and Charlie transfers back to Alice, transfers could be repeated anytime).
1
Clever Turkey Nov 6, 2019 at 20:07
#issue9026

> I assumed that each key pair is used only once

Security checks must not assume a specific user behavior.

> But every time a new domain is registered, -2 category is populated from the sender address and as far as new domains creation is possible only via internal messages

That's a problem. Noone ever can take ownership over non-owned domain, even it expired years ago.
1
D
Deleted Account Oct 18, 2019 at 17:09
Manual dns - messages are accepted before seq_no check: no replay protection
D
Deleted Account Oct 18, 2019 at 17:09
Auto dns - useless (and even harmful, since order should be preserved) seqno check in internal messages
D
Deleted Account Oct 18, 2019 at 17:09
Auto dns - Domains are essentially a numbers and not slices. Thus only domains up to 32 bytes are possible, and anything longer will just fail (no checks).
Manual dns uses 256-bit fixed size keys instead of prefix dicts
Manual DNS: seqno checking is a mess, for PK update it is not checked at all, for new entry it is checked only after accept: unauthenticated reply attack with draining contract balance is possible. Moreover, it WILL happen after message is sent to net if it is not removed by collators - be ready for surprises. Even more, updating public key does not check for seqno and does not change seqno - therefore will be replayed many times meanwhile draining the balance of contract.

Interesting way to slice bytes looking for delimiter, I used that in iteration zero of my contract :)
Automatic DNS: price calculation may not be correct, internal dictionary structure may spawn off far more cells and bits than you expect, seqno checking and processing for internal messages is useless and harmful, but WHERE IT IS NEEDED: in the external message processing (public key and prices update) it is missing - therefore the message may be replayed by accident (collator) or by malicious party: in case the party obtains the signed copy of price update message, he could replay it anytime and play with your contract's prices. Good idea, but implemented bad.
Eager Boar Oct 22, 2019 at 21:37
For the public key update I didn’t check seqno for purpose, after the key is updated, the message won’t be valid any more as it is signed by the old key. As for price updates you’re right, my bad.
Nobody added any issues yet...