Skip to content

Conversation

@joncinque
Copy link
Contributor

@joncinque joncinque commented Sep 29, 2025

Problem

There's just too many ways for signers to be potentially abused during transfer hooks.

Summary of changes

Demote all accounts to non-signer before invoking the transfer hook program. Let me know if you think the test is useless, I just wanted to be sure that the code was correct.

At the same time, remove cdylib target for the interface crate, and remove the lib target for the program crate. NOTE: can't do the second one

cc @tiago18c

#### Problem

There's just too many ways for signers to be potentially abused during
transfer hooks.

#### Summary of changes

Demote all accounts to non-signer before invoking the transfer hook
program.
Copy link

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change itself is fine, but we have a bit of a confusing dependency tree going for transfer-hook, and the leaf (tlv-account-resolution) actually has a method that's supposed to do some of this work under the hood.

https://github.com/solana-program/libraries/blob/788a65685ccfdfd4d33af752b9aadf739626d113/tlv-account-resolution/src/state.rs#L35

Maybe it's best to include this change there instead?

@joncinque
Copy link
Contributor Author

My thinking was that TLV account resolution could be used for other use-cases, outside of transfer hook programs, so that it would be better to fix it only at the level that we were worried about, which was transfer hooks.

After writing it out though, I realize that's kind of silly -- we can always add new functions in TLV account resolution later if needed.

@joncinque joncinque closed this Sep 30, 2025
@joncinque joncinque deleted the demoteall branch September 30, 2025 10:49
joncinque added a commit to joncinque/libraries that referenced this pull request Sep 30, 2025
#### Problem

As described at solana-program/transfer-hook#83,
there's just too many ways for signers to be potentially abused during
transfer hooks.

#### Summary of changes

Demote all accounts to non-signer when resolving from an extra account
metas list.
@buffalojoec
Copy link

After writing it out though, I realize that's kind of silly -- we can always add new functions in TLV account resolution later if needed.

Agreed, we can just hoist deescalate_account_metas out of that library if we ever need to. For now, might as well pile on, since the name is kinda implied already.

joncinque added a commit to solana-program/libraries that referenced this pull request Oct 1, 2025
* tlv-account-resolution: Always demote signer flag

#### Problem

As described at solana-program/transfer-hook#83,
there's just too many ways for signers to be potentially abused during
transfer hooks.

#### Summary of changes

Demote all accounts to non-signer when resolving from an extra account
metas list.

* Review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants