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

Airnode attemps to fail the request when the sponsor wallet is invalid #1749

Closed
Tracked by #1759
bbenligiray opened this issue Apr 24, 2023 · 4 comments · Fixed by #1758
Closed
Tracked by #1759

Airnode attemps to fail the request when the sponsor wallet is invalid #1749

bbenligiray opened this issue Apr 24, 2023 · 4 comments · Fixed by #1758
Assignees
Labels
bug Something needs to be fixed needs documentation
Milestone

Comments

@bbenligiray
Copy link
Member

Run yarn && yarn main on https://github.com/bbenligiray/fulfillment-analysis and you will see a bunch of reverted transactions that attempt to call fail() with the errorMessage Sponsor wallet is invalid.... These fail() calls of course revert because the sponsor wallet is invalid. This is a DoS vector for certain advanced user patterns for RRP (where you let the end-user specify the sponsor wallet).

When the sponsor wallet for a request is invalid, Airnode should just log it and ignore it on-chain.

@bbenligiray bbenligiray changed the title Airnode attemps to fail a request when the sponsor wallet is invalid Airnode attemps to fail the request when the sponsor wallet is invalid Apr 24, 2023
@dcroote
Copy link
Contributor

dcroote commented Apr 25, 2023

I was able to reproduce this by using the wrong sponsor wallet address:
https://sepolia.etherscan.io/tx/0x54478cdca3f585fcb07ed85180ed87eee8401dc2b3008be64f3bf570a499ef2d

The relevant code for the error is below, with RequestErrorMessage.SponsorWalletInvalid being the error message Sponsor wallet is invalid:

export function verifySponsorWallet(payload: RegularApiCallPayload): LogsData<ApiCallErrorResponse> | null {
const { config, aggregatedApiCall } = payload;
const { sponsorAddress, sponsorWalletAddress, id } = aggregatedApiCall;
const derivedSponsorWallet = deriveSponsorWalletFromMnemonic(
config.nodeSettings.airnodeWalletMnemonic,
sponsorAddress
);
if (derivedSponsorWallet.address === sponsorWalletAddress) return null;
// TODO: Abstract this to a logging utils file
const message = `${RequestErrorMessage.SponsorWalletInvalid}, Request ID:${id}`;
const log = logger.pend('ERROR', message);
return [[log], { success: false, errorMessage: message }];
}



When the sponsor wallet for a request is invalid, Airnode should just log it and ignore it on-chain.

It turns out we already do this for withdrawal requests:

export function verifySponsorWallets<T>(
unverifiedRequests: Request<T>[],
masterHDNode: ethers.utils.HDNode
): LogsData<Request<T>[]> {
const { logs, requests } = unverifiedRequests.reduce(
(acc, request) => {
const expectedSponsorWalletAddress = wallet.deriveSponsorWallet(masterHDNode, request.sponsorAddress).address;
if (request.sponsorWalletAddress !== expectedSponsorWalletAddress) {
const message = `Invalid sponsor wallet:${request.sponsorWalletAddress} for Request:${request.id}. Expected:${expectedSponsorWalletAddress}`;
const log = logger.pend('ERROR', message);
return { ...acc, logs: [...acc.logs, log] };
}
const message = `Request ID:${request.id} is linked to a valid sponsor wallet:${request.sponsorWalletAddress}`;
const log = logger.pend('DEBUG', message);
return { ...acc, logs: [...acc.logs, log], requests: [...acc.requests, request] };
},
{ logs: [], requests: [] } as UpdatedRequests<T>
);
return [logs, requests];
}

@aTeoke aTeoke added the bug Something needs to be fixed label Apr 25, 2023
@aTeoke aTeoke modified the milestones: 0.11.0, 0.12.0 Apr 26, 2023
@dcroote dcroote self-assigned this Apr 26, 2023
@aTeoke
Copy link

aTeoke commented May 4, 2023

Just to add, we are only patching the current version 0.11.0 upon agreement on Airnode weekly call(week ago).

@aTeoke aTeoke modified the milestones: 0.12.0, 0.11.1 May 4, 2023
@dcroote
Copy link
Contributor

dcroote commented May 5, 2023

@aTeoke - to test this you can follow the airnode-example coingecko flow, but before making the request, change the sponsor wallet to something random (invalid) in the coingecko/request-utils.ts file:

const sponsorWalletAddress = await deriveSponsorWalletAddress(
deriveAirnodeXpub(airnodeWallet.mnemonic.phrase),
airnodeWallet.address,
sponsor.address
);

Before this PR, the result would be a failed tx like I linked to in my comment above. Now, Airnode ignores the request because the sponsor wallet is invalid and does not make the API call or submit a tx.

@aTeoke
Copy link

aTeoke commented May 10, 2023

Tested it, Derek guided me through it and we can consider this one done and working ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something needs to be fixed needs documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants