Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject Solana transaction which contains malicious instructions #1590

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ enum SolanaErrorCode solana_base64_encode_transaction(const void *txn, const cha

const void *solana_deserialize_transaction(const uint8_t *txn, size_t txn_len);

bool solana_transaction_contains_set_authority(const void *txn);

enum SolanaErrorCode solana_sign_transaction(const void *txn,
const uint8_t *recent_blockhash,
size_t recent_blockhash_len,
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ enum SolanaErrorCode solana_base64_encode_transaction(const void *txn, const cha

const void *solana_deserialize_transaction(const uint8_t *txn, size_t txn_len);

bool solana_transaction_contains_set_authority(const void *txn);

enum SolanaErrorCode solana_sign_transaction(const void *txn,
const uint8_t *recent_blockhash,
size_t recent_blockhash_len,
Expand Down
Binary file not shown.
1 change: 1 addition & 0 deletions Mixin/Resources/en.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@
"lost_your_mobile_number" = "Lost your mobile number?";
"low_24h" = "24H LOW";
"make_group_admin" = "Make group admin";
"malicious_instruction_set_authority" = "This transaction change the owner of your SPL tokens. This allow the new owner to transfer the tokens without your permission.";
"manage" = "Manage";
"manage_your_apps" = "Manage your bots";
"manage_your_circles" = "Manage your circles";
Expand Down
1 change: 1 addition & 0 deletions Mixin/Resources/es.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@
"lost_your_mobile_number" = "¿Has perdido tu número de móvil?";
"low_24h" = "24H LOW";
"make_group_admin" = "Hacer administrador de grupo";
"malicious_instruction_set_authority" = "This transaction change the owner of your SPL tokens. This allow the new owner to transfer the tokens without your permission.";
"manage" = "Administrar";
"manage_your_apps" = "Administra tus bots";
"manage_your_circles" = "Administra tus círculos";
Expand Down
1 change: 1 addition & 0 deletions Mixin/Resources/ja.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@
"lost_your_mobile_number" = "電話番号をお忘れですか?";
"low_24h" = "24H LOW";
"make_group_admin" = "管理者権限を付与";
"malicious_instruction_set_authority" = "This transaction change the owner of your SPL tokens. This allow the new owner to transfer the tokens without your permission.";
"manage" = "管理";
"manage_your_apps" = "Manage your bots";
"manage_your_circles" = "Manage your circles";
Expand Down
1 change: 1 addition & 0 deletions Mixin/Resources/ru.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@
"lost_your_mobile_number" = "Потеряли номер мобильного?";
"low_24h" = "24H LOW";
"make_group_admin" = "Сделать администратором группы";
"malicious_instruction_set_authority" = "This transaction change the owner of your SPL tokens. This allow the new owner to transfer the tokens without your permission.";
"manage" = "Управлять";
"manage_your_apps" = "Manage your bots";
"manage_your_circles" = "Manage your circles";
Expand Down
1 change: 1 addition & 0 deletions Mixin/Resources/zh-Hans.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@
"lost_your_mobile_number" = "手机丢失?";
"low_24h" = "24 小时最低";
"make_group_admin" = "设定为群组管理员";
"malicious_instruction_set_authority" = "此交易将更改 SPL 代币的所有者。这将允许新的所有者在未经您允许的情况下转移代币。";
"manage" = "管理";
"manage_your_apps" = "管理你所有的机器人";
"manage_your_circles" = "管理你所有的圈子";
Expand Down
1 change: 1 addition & 0 deletions Mixin/Resources/zh-Hant.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@
"lost_your_mobile_number" = "手機丟失?";
"low_24h" = "24 小時最低";
"make_group_admin" = "設定為群組管理員";
"malicious_instruction_set_authority" = "此交易將更改 SPL 代幣的所有者。這將允許新的所有者在未經您允許的情況下轉移代幣。";
"manage" = "管理";
"manage_your_apps" = "管理你所有的機器人";
"manage_your_circles" = "管理你所有的圈子";
Expand Down
4 changes: 4 additions & 0 deletions Mixin/Service/Web3/SolanaTransferOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ class ArbitraryTransactionSolanaTransferOperation: SolanaTransferOperation {
Logger.web3.info(category: "SolanaTransfer", message: "Rejected")
}

func transactionContainsSetAuthority() -> Bool {
transaction.containsSetAuthority()
}

}

final class SolanaTransferWithWalletConnectOperation: ArbitraryTransactionSolanaTransferOperation {
Expand Down
4 changes: 4 additions & 0 deletions Mixin/UserInterface/Controllers/Web3/Model/Solana.swift
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ extension Solana {
solana_free_transaction(pointer)
}

func containsSetAuthority() -> Bool {
solana_transaction_contains_set_authority(pointer)
}

func sign(withPrivateKeyFrom seed: Data, recentBlockhash: Data) throws -> String {
try recentBlockhash.withUnsafeBytes { recentBlockhash in
try seed.withUnsafeBytes { seed in
Expand Down
167 changes: 94 additions & 73 deletions Mixin/UserInterface/Controllers/Web3/Web3TransferViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,26 @@ final class Web3TransferViewController: AuthenticationPreviewViewController {
override func viewDidLoad() {
super.viewDidLoad()

tableHeaderView.setIcon { imageView in
if let operation = operation as? Web3TransferWithWalletConnectOperation {
imageView.sd_setImage(with: operation.session.iconURL)
} else {
switch proposer {
case .dapp, .none:
imageView.image = R.image.unknown_session()
case .web3ToMixinWallet, .web3ToAddress:
imageView.image = R.image.web3_sign_transfer()
let operationContainsSetAuthority: Bool
if let operation = operation as? ArbitraryTransactionSolanaTransferOperation {
operationContainsSetAuthority = operation.transactionContainsSetAuthority()
} else {
operationContainsSetAuthority = false
}

if operationContainsSetAuthority {
tableHeaderView.setIcon(progress: .failure)
} else {
tableHeaderView.setIcon { imageView in
if let operation = operation as? Web3TransferWithWalletConnectOperation {
imageView.sd_setImage(with: operation.session.iconURL)
} else {
switch proposer {
case .dapp, .none:
imageView.image = R.image.unknown_session()
case .web3ToMixinWallet, .web3ToAddress:
imageView.image = R.image.web3_sign_transfer()
}
}
}
}
Expand All @@ -60,28 +71,34 @@ final class Web3TransferViewController: AuthenticationPreviewViewController {
} else {
R.string.localizable.signature_request()
}
switch proposer {
case .dapp, .none:
layoutTableHeaderView(title: title,
subtitle: R.string.localizable.web3_signing_warning(),
style: .destructive)
case .web3ToMixinWallet, .web3ToAddress:
layoutTableHeaderView(title: title,
subtitle: R.string.localizable.signature_request_from(mixinMessenger),
style: [])
if operationContainsSetAuthority {
let subtitle = R.string.localizable.malicious_instruction_set_authority()
layoutTableHeaderView(title: title, subtitle: subtitle, style: [])
} else {
switch proposer {
case .dapp, .none:
let subtitle = R.string.localizable.web3_signing_warning()
layoutTableHeaderView(title: title, subtitle: subtitle, style: .destructive)
case .web3ToMixinWallet, .web3ToAddress:
let subtitle = R.string.localizable.signature_request_from(mixinMessenger)
layoutTableHeaderView(title: title, subtitle: subtitle, style: [])
}
}

var rows: [Row] = [
.web3Message(caption: R.string.localizable.estimated_balance_change(),
message: R.string.localizable.loading())
]
rows.append(
.amount(caption: .fee,
token: R.string.localizable.calculating(),
fiatMoney: R.string.localizable.calculating(),
display: .byToken,
boldPrimaryAmount: false)
)
var rows: [Row]
if operationContainsSetAuthority {
rows = []
} else {
rows = [
.web3Message(caption: R.string.localizable.estimated_balance_change(),
message: R.string.localizable.loading()),
.amount(caption: .fee,
token: R.string.localizable.calculating(),
fiatMoney: R.string.localizable.calculating(),
display: .byToken,
boldPrimaryAmount: false)
]
}

switch proposer {
case .dapp(let proposer):
Expand All @@ -103,55 +120,59 @@ final class Web3TransferViewController: AuthenticationPreviewViewController {
rows.append(.info(caption: .network, content: operation.chain.name))
reloadData(with: rows)

stateObserver = operation.$state.sink { [weak self] state in
self?.reloadData(state: state)
}
reloadData(state: operation.state)

Task { [operation, weak self] in
do {
let change = try await operation.loadBalanceChange()
await MainActor.run {
let row: Row
switch change {
case let .decodingFailed(rawTransaction):
row = .web3Message(caption: R.string.localizable.transaction(),
message: rawTransaction)
case let .detailed(token, amount):
let tokenAmount = CurrencyFormatter.localizedString(from: amount, format: .precision, sign: .never)
let fiatMoneyValue = amount * token.decimalUSDPrice * Currency.current.decimalRate
let fiatMoneyAmount = CurrencyFormatter.localizedString(from: fiatMoneyValue, format: .fiatMoney, sign: .never, symbol: .currencySymbol)
row = .web3Amount(caption: R.string.localizable.estimated_balance_change(),
tokenAmount: tokenAmount,
fiatMoneyAmount: fiatMoneyAmount,
token: token)
}
self?.replaceRow(at: 0, with: row)
}
} catch {
Logger.web3.error(category: "Web3TransferView", message: "Load bal. change: \(error)")
if operationContainsSetAuthority {
loadSingleButtonTrayView(title: R.string.localizable.reject(), action: #selector(close(_:)))
} else {
stateObserver = operation.$state.sink { [weak self] state in
self?.reloadData(state: state)
}
do {
if let fee = try await operation.loadFee() {
reloadData(state: operation.state)

Task { [operation, weak self] in
do {
let change = try await operation.loadBalanceChange()
await MainActor.run {
let feeValue = CurrencyFormatter.localizedString(from: fee.token, format: .networkFee, sign: .never, symbol: nil)
let feeCost = if fee.fiatMoney >= 0.01 {
CurrencyFormatter.localizedString(from: fee.fiatMoney, format: .fiatMoney, sign: .never, symbol: .currencySymbol)
} else {
"<" + CurrencyFormatter.localizedString(from: 0.01, format: .fiatMoney, sign: .never, symbol: .currencySymbol)
let row: Row
switch change {
case let .decodingFailed(rawTransaction):
row = .web3Message(caption: R.string.localizable.transaction(),
message: rawTransaction)
case let .detailed(token, amount):
let tokenAmount = CurrencyFormatter.localizedString(from: amount, format: .precision, sign: .never)
let fiatMoneyValue = amount * token.decimalUSDPrice * Currency.current.decimalRate
let fiatMoneyAmount = CurrencyFormatter.localizedString(from: fiatMoneyValue, format: .fiatMoney, sign: .never, symbol: .currencySymbol)
row = .web3Amount(caption: R.string.localizable.estimated_balance_change(),
tokenAmount: tokenAmount,
fiatMoneyAmount: fiatMoneyAmount,
token: token)
}
let row: Row = .amount(caption: .fee,
token: feeValue + " " + operation.feeToken.symbol,
fiatMoney: feeCost,
display: .byToken,
boldPrimaryAmount: false)
self?.replaceRow(at: 1, with: row)
self?.replaceRow(at: 0, with: row)
}
} else {
Logger.web3.info(category: "Web3TransferView", message: "Unable to load fee")
} catch {
Logger.web3.error(category: "Web3TransferView", message: "Load bal. change: \(error)")
}
do {
if let fee = try await operation.loadFee() {
await MainActor.run {
let feeValue = CurrencyFormatter.localizedString(from: fee.token, format: .networkFee, sign: .never, symbol: nil)
let feeCost = if fee.fiatMoney >= 0.01 {
CurrencyFormatter.localizedString(from: fee.fiatMoney, format: .fiatMoney, sign: .never, symbol: .currencySymbol)
} else {
"<" + CurrencyFormatter.localizedString(from: 0.01, format: .fiatMoney, sign: .never, symbol: .currencySymbol)
}
let row: Row = .amount(caption: .fee,
token: feeValue + " " + operation.feeToken.symbol,
fiatMoney: feeCost,
display: .byToken,
boldPrimaryAmount: false)
self?.replaceRow(at: 1, with: row)
}
} else {
Logger.web3.info(category: "Web3TransferView", message: "Unable to load fee")
}
} catch {
Logger.web3.error(category: "Web3TransferView", message: "Load fee: \(error)")
}
} catch {
Logger.web3.error(category: "Web3TransferView", message: "Load fee: \(error)")
}
}
}
Expand Down