Skip to content
Merged
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 @@ -13,6 +13,8 @@ import {
TransactionOutput,
} from "@/wasm/core/kaspa";
import { calcRevealInputMass } from "@/lib/kaspaFee";
import { hasPartialOutputCommitment } from "@/lib/wallet/sign-script.ts";
import { PARTIAL_OUTPUT_WARNING } from "@/components/screens/browser-api/kaspa/sign-tx/SignTx";

type SignAndBroadcastProps = {
wallet: IWallet;
Expand Down Expand Up @@ -156,9 +158,16 @@ export default function SignAndBroadcast({
cancel={handleCancel}
confirm={handleConfirm}
warning={
feeAdjusted
? "Transaction fee was insufficient. Network fee has been automatically adjusted."
: undefined
[
hasPartialOutputCommitment(payload.scripts)
? PARTIAL_OUTPUT_WARNING
: undefined,
feeAdjusted
? "Transaction fee was insufficient. Network fee has been automatically adjusted."
: undefined,
]
.filter(Boolean)
.join("\n\n") || undefined
}
/>
);
Expand Down
9 changes: 9 additions & 0 deletions components/screens/browser-api/kaspa/sign-tx/SignTx.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import SignConfirm from "@/components/screens/browser-api/kaspa/sign/SignConfirm
import { ApiUtils } from "@/api/background/utils";
import useAnalytics from "@/hooks/useAnalytics";
import { deserializeTransaction } from "@/lib/kaspa-compat";
import { hasPartialOutputCommitment } from "@/lib/wallet/sign-script.ts";

export const PARTIAL_OUTPUT_WARNING =
"This request signs with a sighash type that commits to only part of the transaction outputs. The outputs it does not cover are not protected by your signature and can still be changed after you approve.";

type SignTxProps = {
wallet: IWallet;
Expand Down Expand Up @@ -65,6 +69,11 @@ export default function SignTx({
confirm={handleConfirm}
cancel={handleCancel}
payload={payload}
warning={
hasPartialOutputCommitment(payload.scripts)
? PARTIAL_OUTPUT_WARNING
: undefined
}
/>
</>
);
Expand Down
2 changes: 1 addition & 1 deletion components/screens/browser-api/kaspa/sign/SignConfirm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ export default function SignConfirm({
{!networkMismatched && (
<>
{warning && (
<div className="mt-3 rounded-lg border border-yellow-600 bg-yellow-900/30 px-4 py-2 text-xs text-yellow-400">
<div className="mt-3 whitespace-pre-line rounded-lg border border-yellow-600 bg-yellow-900/30 px-4 py-2 text-xs text-yellow-400">
{warning}
</div>
)}
Expand Down
7 changes: 5 additions & 2 deletions hooks/wallet/useKaspaLedgerSigner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,19 @@ export default function useKaspaLedgerSigner() {
? new LegacyAccountFactory()
: new AccountFactory();

// the signer and getAddress must derive from the same index, otherwise the
// address shown to the user is not the key that signs
const accountIndex = account?.index ?? 0;

const getAddress = async () => {
const accountIndex = account?.index ?? 0;
const publicKey = await factory
.createFromLedger(transport, accountIndex)
.getPublicKey();
const address = publicKey.toAddress(networkId).toString();
return address;
};

const signer = factory.createFromLedger(transport);
const signer = factory.createFromLedger(transport, accountIndex);

return {
getAddress,
Expand Down
52 changes: 51 additions & 1 deletion lib/wallet/sign-script.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,51 @@ export type RawScriptOption = Partial<ScriptOption> & { script?: string };

const HEX_RE = /^(?:[0-9a-fA-F]{2})*$/;

// Sighash types split on one axis: how much of the output set the signature
// commits to.
// All / AllAnyOneCanPay -> every output committed (safe)
// Single / SingleAnyOneCanPay -> only the same-index output (partial)
// None / NoneAnyOneCanPay -> no outputs committed at all (unsafe)
// With None*, the amount and destination the confirm screen showed are not
// bound by the signature and can be rewritten after approval, so they are
// refused outright.
// TODO: allow per-request opt-in from an advanced setting instead of a
// hardcoded constant, once that setting exists.
const ALLOW_UNSAFE_OUTPUT_SIGHASH = false;

const UNSAFE_OUTPUT_SIGN_TYPES = ["None", "NoneAnyOneCanPay"] as const;
const PARTIAL_OUTPUT_SIGN_TYPES = ["Single", "SingleAnyOneCanPay"] as const;

// Chokepoint for the None* refusal: called from normalizeScriptOptions (fails
// the whole request early, before any mutation) AND from
// signTxInputWithScriptOption itself, so a future caller that skips
// normalization cannot silently reintroduce unsafe signing.
function assertSafeOutputSighash(signType: string, inputIndex: number): void {
if (
!ALLOW_UNSAFE_OUTPUT_SIGHASH &&
(UNSAFE_OUTPUT_SIGN_TYPES as readonly string[]).includes(signType)
) {
throw new Error(
`signTx: signType "${signType}" commits no outputs, so every output could be rewritten after signing; refusing to sign input ${inputIndex}`,
);
}
}

/**
* True when any option signs with a sighash type that commits only part of the
* outputs, so the rest can still change after the user approves. Non-throwing:
* safe to call from render paths that also display invalid requests.
*/
export function hasPartialOutputCommitment(
scripts?: (RawScriptOption | null | undefined)[],
): boolean {
return (scripts ?? []).some((option) =>
(PARTIAL_OUTPUT_SIGN_TYPES as readonly string[]).includes(
option?.signType ?? "All",
),
);
}

export function normalizeScriptOptions(
scripts?: (RawScriptOption | null | undefined)[],
): ScriptOption[] {
Expand All @@ -38,7 +83,10 @@ export function normalizeScriptOptions(
throw new Error(`signTx: invalid scriptHex for input ${inputIndex}`);
}

return { inputIndex, scriptHex, signType: option.signType ?? "All" };
const signType = option.signType ?? "All";
assertSafeOutputSighash(signType, inputIndex);

return { inputIndex, scriptHex, signType };
});
}

Expand Down Expand Up @@ -71,6 +119,8 @@ export function signTxInputWithScriptOption(
option: ScriptOption,
privateKeyString: string,
) {
assertSafeOutputSighash(option.signType ?? "All", option.inputIndex);

// each tx.inputs access crosses the WASM boundary; read it once
const inputs = tx.inputs;
if (option.inputIndex >= inputs.length) {
Comment on lines +122 to 126
Expand Down
Loading
Loading