Dargon789 / safe-wallet-web

Safe{Wallet} – multisig EVM wallet
https://app.safe.global
GNU General Public License v3.0
1 stars 1 forks source link

Fix code scanning alert no. 3: Server-side request forgery #8

Closed Dargon789 closed 2 weeks ago

Dargon789 commented 2 weeks ago

Fixes https://github.com/Dargon789/safe-wallet-web/security/code-scanning/3

To fix the SSRF vulnerability, we should avoid using user input directly in the hostname of the URL. Instead, we can use an allow-list of known safe URLs or domains. This ensures that only pre-approved URLs can be used in the request, mitigating the risk of SSRF.

  1. Create an allow-list of known safe URLs or domains.
  2. Validate the appUrl against this allow-list before making the request.
  3. If the appUrl is not in the allow-list, throw an error or handle it appropriately.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

github-actions[bot] commented 2 weeks ago

Branch preview

❌ Deploy failed!

github-actions[bot] commented 2 weeks ago

Coverage report

[!CAUTION] Test run failed

St.:grey_question:
Category Percentage Covered / Total
🟑 Statements
73.5% (-0.03% πŸ”»)
13493/18359
πŸ”΄ Branches
51.34% (-0.01% πŸ”»)
3282/6393
πŸ”΄ Functions 56.99% 1960/3439
🟑 Lines
75.19% (-0.04% πŸ”»)
12269/16318
Show files with reduced coverage πŸ”»
|
St.:grey_question:
| File | Statements | Branches | Functions | Lines | | :-: | :- | :- | :- | :- | :- | | πŸ”΄ |
`...` / manifest.ts
|
34.04% (-7.82% πŸ”»)
|
8% (+3.83% πŸ”Ό)
| 28.57% |
38.1% (-9.27% πŸ”»)
| | 🟒 |
`...` / useTxPendingStatuses.ts
|
83.64% (-0.91% πŸ”»)
|
68.33% (-1.67% πŸ”»)
| 75% |
92.63% (-1.05% πŸ”»)
|

Test suite run failed

Failed tests: 5/1573. Failed suites: 2/208.
``` ● useApprovalInfos β€Ί returns an ApprovalInfo if the transaction contains an increaseAllowance call TypeError: unknown function (argument="fragment", value="increaseAllowance", code=INVALID_ARGUMENT, version=6.11.1) 50 | 51 | if (txPartial.data.startsWith(INCREASE_ALLOWANCE_SIGNATURE_HASH)) { > 52 | const [spender, amount] = ERC20_INTERFACE.decodeFunctionData('increaseAllowance', txPartial.data) | ^ 53 | return [ 54 | { 55 | amount, at makeError (node_modules/ethers/src.ts/utils/errors.ts:687:21) at assert (node_modules/ethers/src.ts/utils/errors.ts:715:25) at assertArgument (node_modules/ethers/src.ts/utils/errors.ts:727:5) at Interface.decodeFunctionData (node_modules/ethers/src.ts/abi/interface.ts:860:27) at Function.decodeFunctionData [as scanInnerTransaction] (src/services/security/modules/ApprovalModule/index.ts:52:49) at ApprovalModule.scanInnerTransaction [as scanTransaction] (src/services/security/modules/ApprovalModule/index.ts:126:44) at scanTransaction (src/components/tx/ApprovalEditor/hooks/useApprovalInfos.ts:39:37) at mountMemo (node_modules/react-dom/cjs/react-dom.development.js:16406:19) at Object.useMemo (node_modules/react-dom/cjs/react-dom.development.js:16851:16) at useMemo (node_modules/react/cjs/react.development.js:1650:21) at useApprovalInfos (src/components/tx/ApprovalEditor/hooks/useApprovalInfos.ts:37:28) at src/components/tx/ApprovalEditor/hooks/useApprovalInfos.test.ts:88:57 at TestComponent (node_modules/@testing-library/react/dist/pure.js:283:27) at renderWithHooks (node_modules/react-dom/cjs/react-dom.development.js:15486:18) at mountIndeterminateComponent (node_modules/react-dom/cjs/react-dom.development.js:20103:13) at beginWork (node_modules/react-dom/cjs/react-dom.development.js:21626:16) at beginWork$1 (node_modules/react-dom/cjs/react-dom.development.js:27465:14) at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:26599:12) at workLoopSync (node_modules/react-dom/cjs/react-dom.development.js:26505:5) at renderRootSync (node_modules/react-dom/cjs/react-dom.development.js:26473:7) at recoverFromConcurrentError (node_modules/react-dom/cjs/react-dom.development.js:25889:20) at performConcurrentWorkOnRoot (node_modules/react-dom/cjs/react-dom.development.js:25789:22) at flushActQueue (node_modules/react/cjs/react.development.js:2667:24) at act (node_modules/react/cjs/react.development.js:2582:11) at actWithWarning (node_modules/react-dom/cjs/react-dom-test-utils.development.js:1740:10) at node_modules/@testing-library/react/dist/act-compat.js:63:25 at renderRoot (node_modules/@testing-library/react/dist/pure.js:159:26) at render (node_modules/@testing-library/react/dist/pure.js:246:10) at renderHook (node_modules/@testing-library/react/dist/pure.js:293:7) at customRenderHook (src/tests/test-utils.tsx:85:20) at Object. (src/components/tx/ApprovalEditor/hooks/useApprovalInfos.test.ts:88:34) ● useApprovalInfos β€Ί returns multiple ApprovalInfos if the transaction is a multiSend containing an approve and increaseAllowance call TypeError: unknown function (argument="fragment", value="increaseAllowance", code=INVALID_ARGUMENT, version=6.11.1) 50 | 51 | if (txPartial.data.startsWith(INCREASE_ALLOWANCE_SIGNATURE_HASH)) { > 52 | const [spender, amount] = ERC20_INTERFACE.decodeFunctionData('increaseAllowance', txPartial.data) | ^ 53 | return [ 54 | { 55 | amount, at makeError (node_modules/ethers/src.ts/utils/errors.ts:687:21) at assert (node_modules/ethers/src.ts/utils/errors.ts:715:25) at assertArgument (node_modules/ethers/src.ts/utils/errors.ts:727:5) at Interface.decodeFunctionData (node_modules/ethers/src.ts/abi/interface.ts:860:27) at Function.decodeFunctionData [as scanInnerTransaction] (src/services/security/modules/ApprovalModule/index.ts:52:49) at scanInnerTransaction (src/services/security/modules/ApprovalModule/index.ts:124:76) at Array.flatMap () at ApprovalModule.flatMap [as scanTransaction] (src/services/security/modules/ApprovalModule/index.ts:124:38) at scanTransaction (src/components/tx/ApprovalEditor/hooks/useApprovalInfos.ts:39:37) at mountMemo (node_modules/react-dom/cjs/react-dom.development.js:16406:19) at Object.useMemo (node_modules/react-dom/cjs/react-dom.development.js:16851:16) at useMemo (node_modules/react/cjs/react.development.js:1650:21) at useApprovalInfos (src/components/tx/ApprovalEditor/hooks/useApprovalInfos.ts:37:28) at src/components/tx/ApprovalEditor/hooks/useApprovalInfos.test.ts:134:57 at TestComponent (node_modules/@testing-library/react/dist/pure.js:283:27) at renderWithHooks (node_modules/react-dom/cjs/react-dom.development.js:15486:18) at mountIndeterminateComponent (node_modules/react-dom/cjs/react-dom.development.js:20103:13) at beginWork (node_modules/react-dom/cjs/react-dom.development.js:21626:16) at beginWork$1 (node_modules/react-dom/cjs/react-dom.development.js:27465:14) at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:26599:12) at workLoopSync (node_modules/react-dom/cjs/react-dom.development.js:26505:5) at renderRootSync (node_modules/react-dom/cjs/react-dom.development.js:26473:7) at recoverFromConcurrentError (node_modules/react-dom/cjs/react-dom.development.js:25889:20) at performConcurrentWorkOnRoot (node_modules/react-dom/cjs/react-dom.development.js:25789:22) at flushActQueue (node_modules/react/cjs/react.development.js:2667:24) at act (node_modules/react/cjs/react.development.js:2582:11) at actWithWarning (node_modules/react-dom/cjs/react-dom-test-utils.development.js:1740:10) at node_modules/@testing-library/react/dist/act-compat.js:63:25 at renderRoot (node_modules/@testing-library/react/dist/pure.js:159:26) at render (node_modules/@testing-library/react/dist/pure.js:246:10) at renderHook (node_modules/@testing-library/react/dist/pure.js:293:7) at customRenderHook (src/tests/test-utils.tsx:85:20) at Object. (src/components/tx/ApprovalEditor/hooks/useApprovalInfos.test.ts:134:34) ● useApprovalInfos β€Ί returns multiple ApprovalInfos if the transaction is a multiSend containing 2 approvals and other transaction inbetween TypeError: unknown function (argument="fragment", value="increaseAllowance", code=INVALID_ARGUMENT, version=6.11.1) 50 | 51 | if (txPartial.data.startsWith(INCREASE_ALLOWANCE_SIGNATURE_HASH)) { > 52 | const [spender, amount] = ERC20_INTERFACE.decodeFunctionData('increaseAllowance', txPartial.data) | ^ 53 | return [ 54 | { 55 | amount, at makeError (node_modules/ethers/src.ts/utils/errors.ts:687:21) at assert (node_modules/ethers/src.ts/utils/errors.ts:715:25) at assertArgument (node_modules/ethers/src.ts/utils/errors.ts:727:5) at Interface.decodeFunctionData (node_modules/ethers/src.ts/abi/interface.ts:860:27) at Function.decodeFunctionData [as scanInnerTransaction] (src/services/security/modules/ApprovalModule/index.ts:52:49) at scanInnerTransaction (src/services/security/modules/ApprovalModule/index.ts:124:76) at Array.flatMap () at ApprovalModule.flatMap [as scanTransaction] (src/services/security/modules/ApprovalModule/index.ts:124:38) at scanTransaction (src/components/tx/ApprovalEditor/hooks/useApprovalInfos.ts:39:37) at mountMemo (node_modules/react-dom/cjs/react-dom.development.js:16406:19) at Object.useMemo (node_modules/react-dom/cjs/react-dom.development.js:16851:16) at useMemo (node_modules/react/cjs/react.development.js:1650:21) at useApprovalInfos (src/components/tx/ApprovalEditor/hooks/useApprovalInfos.ts:37:28) at src/components/tx/ApprovalEditor/hooks/useApprovalInfos.test.ts:209:57 at TestComponent (node_modules/@testing-library/react/dist/pure.js:283:27) at renderWithHooks (node_modules/react-dom/cjs/react-dom.development.js:15486:18) at mountIndeterminateComponent (node_modules/react-dom/cjs/react-dom.development.js:20103:13) at beginWork (node_modules/react-dom/cjs/react-dom.development.js:21626:16) at beginWork$1 (node_modules/react-dom/cjs/react-dom.development.js:27465:14) at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:26599:12) at workLoopSync (node_modules/react-dom/cjs/react-dom.development.js:26505:5) at renderRootSync (node_modules/react-dom/cjs/react-dom.development.js:26473:7) at recoverFromConcurrentError (node_modules/react-dom/cjs/react-dom.development.js:25889:20) at performConcurrentWorkOnRoot (node_modules/react-dom/cjs/react-dom.development.js:25789:22) at flushActQueue (node_modules/react/cjs/react.development.js:2667:24) at act (node_modules/react/cjs/react.development.js:2582:11) at actWithWarning (node_modules/react-dom/cjs/react-dom-test-utils.development.js:1740:10) at node_modules/@testing-library/react/dist/act-compat.js:63:25 at renderRoot (node_modules/@testing-library/react/dist/pure.js:159:26) at render (node_modules/@testing-library/react/dist/pure.js:246:10) at renderHook (node_modules/@testing-library/react/dist/pure.js:293:7) at customRenderHook (src/tests/test-utils.tsx:85:20) at Object. (src/components/tx/ApprovalEditor/hooks/useApprovalInfos.test.ts:209:34) ``` --- ``` ● ApprovalEditor β€Ί should modify approvals on save TypeError: unknown function (argument="fragment", value="increaseAllowance", code=INVALID_ARGUMENT, version=6.11.1) 156 | { 157 | to: tokenAddress, > 158 | data: ERC20_INTERFACE.encodeFunctionData('increaseAllowance', [spenderAddress, '690000000000']), | ^ 159 | value: '0', 160 | operation: OperationType.Call, 161 | }, at makeError (node_modules/ethers/src.ts/utils/errors.ts:687:21) at assert (node_modules/ethers/src.ts/utils/errors.ts:715:25) at assertArgument (node_modules/ethers/src.ts/utils/errors.ts:727:5) at Interface.encodeFunctionData (node_modules/ethers/src.ts/abi/interface.ts:878:27) at Object.encodeFunctionData (src/components/tx/ApprovalEditor/ApprovalEditor.test.tsx:158:35) ● ApprovalEditor β€Ί should modify increaseAllowance on save TypeError: unknown function (argument="fragment", value="increaseAllowance", code=INVALID_ARGUMENT, version=6.11.1) 264 | { 265 | to: tokenAddress, > 266 | data: ERC20_INTERFACE.encodeFunctionData('increaseAllowance', [spenderAddress, '690000000000']), | ^ 267 | value: '0', 268 | operation: OperationType.Call, 269 | }, at makeError (node_modules/ethers/src.ts/utils/errors.ts:687:21) at assert (node_modules/ethers/src.ts/utils/errors.ts:715:25) at assertArgument (node_modules/ethers/src.ts/utils/errors.ts:727:5) at Interface.encodeFunctionData (node_modules/ethers/src.ts/abi/interface.ts:878:27) at Object.encodeFunctionData (src/components/tx/ApprovalEditor/ApprovalEditor.test.tsx:266:35) ```

Report generated by πŸ§ͺjest coverage report action from 970ccd08be4a50a99d0937e5c2c6af9998b5212e