Skip to content

Commit

Permalink
Merge pull request #10 from CoinFabrik/arturoBeccar-json-corrections
Browse files Browse the repository at this point in the history
Correct missing fields in jsons
  • Loading branch information
aurecoinfabrik authored Nov 4, 2024
2 parents bd81bc2 + c15e61e commit 9203735
Show file tree
Hide file tree
Showing 3 changed files with 1,689 additions and 1,772 deletions.
27 changes: 15 additions & 12 deletions audited-projects/1-Parallel/findings-1-Parallel.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,28 +83,28 @@
"repository": "https://github.com/parallel-finance/parallel",
"audited_commit": "5ca8e13b7b4312855ae2ef1d39f14b38088dfdbd",
"reported_remediated_commit": null,
"file_path": "pallets/liquid-staking/src/lib.rs",
"location": [
{
"file_path": "pallets/liquid-staking/src/lib.rs",
"lines": [{ "from": 1071, "to": 1159 }]
}
],
"reported_impact": "Low",
"reported_likelihood": "Difficulty-High",
"cwe_classification": null,
"vulnerability_class_audit": "Data Validation",
"vulnerable_components": [
"notification_received",
"do_notification_received"
],
"lines": [{ "from": 1071, "to": 1159 }],
"description": "5. Failed XCM requests left in storage\nSeverity: Low\nDifficulty: High\nType: Data Validation\nFinding ID: TOB-PLF-5\nTarget: pallets/liquid-staking/src/lib.rs\nDescription\nWhen the liquid-staking pallet generates an XCM request for the parent chain, the corresponding XCM response triggers a call to Pallet::notification_received. If the response is of the Response::ExecutionResult type, this method calls Pallet::do_notification_received to handle the result.\nThe Pallet::do_notification_received method checks whether the request was successful and then updates the local state according to the corresponding XCM request, which is obtained from the XcmRequests storage map.\nfn do_notification_received(\nquery_id: QueryId,\nrequest: XcmRequest<T>,\nres: Option<(u32, XcmError)>,\n) -> DispatchResult {\nuse ArithmeticKind::*;\nuse XcmRequest::*;\nlet executed = res.is_none();\nif !executed {\nreturn Ok(());\n}\nmatch request {\nBond {\nindex: derivative_index,\namount,\n} => {\nensure!(\n!StakingLedgers::<T>::contains_key(&derivative_index),\nError::<T>::AlreadyBonded\n);\nlet staking_ledger =\n<StakingLedger<T::AccountId, BalanceOf<T>>>::new(\nSelf::derivative_sovereign_account_id(derivative_index),\namount,\n);\nStakingLedgers::<T>::insert(derivative_index, staking_ledger);\nMatchingPool::<T>::try_mutate(|p| -> DispatchResult {\np.update_total_stake_amount(amount, Subtraction)\n})?;\nT::Assets::burn_from(\nSelf::staking_currency()?,\n&Self::account_id(),\nAmount\n)?;\n}\n// ... <redacted>\n}\nXcmRequests::<T>::remove(&query_id);\nOk(())\n}\nFigure 5.1: pallets/liquid-staking/src/lib.rs:1071-1159\nIf the method completes without errors, the XCM request is removed from storage via a call to XcmRequests<T>::remove(query_id). However, if any of the following conditions are true, the corresponding XCM request is left in storage indefinitely:\n1. The request fails and Pallet::do_notification_received exits early.\n2. Pallet::do_notification_received fails.\n3. The response type is not Response::ExecutionResult.\nThese three cases are currently unhandled by the codebase. The same issue is present in the crowdloans pallet implementation of Pallet::do_notification_received.\nRecommendations\nShort term, ensure that failed XCM requests are handled correctly by the crowdloans and liquid-staking pallets.",
"description_summary": "Failed XCM requests are left in storage indefinitely if not handled properly by the notification_received method."
},
{
"title": "Failed XCM requests left in storage",
"title": "Risk of using stale oracle prices in loans pallet",
"repository": "https://github.com/parallel-finance/parallel",
"audited_commit": "5ca8e13b7b4312855ae2ef1d39f14b38088dfdbd",
"reported_remediated_commit": null,
"location": [
{
"file_path": "pallets/liquid-staking/src/lib.rs",
"lines": [{ "from": 1071, "to": 1159 }]
"file_path": "pallets/loans/src/lib.rs",
"lines": [{ "from": 1430, "to": 1441 }]
}
],
"reported_impact": "Low",
Expand Down Expand Up @@ -158,13 +158,16 @@
"repository": "https://github.com/parallel-finance/parallel",
"audited_commit": "5ca8e13b7b4312855ae2ef1d39f14b38088dfdbd",
"reported_remediated_commit": null,
"file_path": "pallets/crowdloans/src/lib.rs",
"location": [
{
"file_path": "pallets/crowdloans/src/lib.rs",
"lines": [{ "from": 502, "to": 594 }]
}
],
"reported_impact": "Informational",
"reported_likelihood": "Difficulty-High",
"cwe_classification": null,
"vulnerability_class_audit": "Data Validation",
"vulnerable_components": ["contribute"],
"lines": [{ "from": 502, "to": 594 }],
"description": "9. The referral code is a sequence of arbitrary bytes\nSeverity: Informational\nDifficulty: High\nType: Data Validation\nFinding ID: TOB-PLF-9\nTarget: pallets/crowdloans/src/lib.rs\nDescription\nThe referral code is used in a number of extrinsic calls in the crowdloans pallet. Because the referral code is never validated, it can be a sequence of arbitrary bytes. The referral code is logged by a number of extrinsics. However, it is currently impossible to perform log injection because the referral code is printed as a hexadecimal string rather than raw bytes (using the debug representation).\npub fn contribute(\norigin: OriginFor<T>,\ncrowdloan: ParaId,\n#[pallet::compact] amount: BalanceOf<T>,\nreferral_code: Vec<u8>,\n) -> DispatchResultWithPostInfo {\n// ... <redacted>\nlog::trace!(\ntarget: \"crowdloans::contribute\",\n\"who: {:?}, para_id: {:?}, amount: {:?}, referral_code: {:?}\",\n&who,\n&crowdloan,\n&amount,\n&referral_code\n);\nOk(().into())\n}\nFigure 9.1: pallets/crowdloans/src/lib.rs:502-594\nExploit Scenario\nThe referral code is rendered as raw bytes in a vulnerable environment, introducing an opportunity to perform a log injection attack.\nRecommendations\nShort term, choose and implement a data type that models the referral code semantics as closely as possible.",
"description_summary": "The referral code in the crowdloans pallet is arbitrary and unvalidated, potentially allowing log injection attacks."
},
Expand Down
8 changes: 6 additions & 2 deletions audited-projects/4-Pendulum/findings-4-Pendulum.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@
"reported_impact": null,
"reported_likelihood": null,
"cwe_classification": null,
"vulnerability_class_audit": "Linters"
"vulnerability_class_audit": "Linters",
"description": "Linter Warnings\nThe codebase has generated several warnings when analyzed using cargo clippy, indicating areas where improvements can be made to enhance the code quality.\n\nID: PDM-002\nScope: Linters\nStatus: Acknowledged\n\nDescription:\nDuring the static analysis process using cargo clippy, the following warnings within the scope of the audit were identified:\n- //rust-lang.github.io/rust-clippy/master/index.html#needless_return\">needless_return\n- //rust-lang.github.io/rust-clippy/master/index.html#needless_borrow\">needless_borrow\n\nLocation:\npallets/orml-currencies-allowance-extension/src/lib.rs:245-246:\n&owner,\n&destination,\n\nRecommendation:\nTo maintain a high-quality and easily maintainable codebase, it is advised to address the warnings generated by cargo clippy. Resolving these linter warnings will improve the overall code quality, facilitate troubleshooting, and potentially enhance performance and security within the project.",
"description_summary": "Codebase has linter warnings that impact maintainability and should be resolved to enhance quality and security."
},
{
"title": "Logging in Runtime",
Expand Down Expand Up @@ -186,7 +188,9 @@
"reported_impact": null,
"reported_likelihood": null,
"cwe_classification": null,
"vulnerability_class_audit": "Code Quality / Testing"
"vulnerability_class_audit": "Code Quality / Testing",
"description": "Test Coverage\nThe current test coverage of the orml-currencies-allowance-extension pallet is insufficient, with only 22.22% coverage. Moreover, the implementation of ChainExtension in the runtime lacks any test coverage.\n\nID: PDM-003\nScope: Code Quality / Testing\nStatus: Acknowledged\n\nDescription:\nTo evaluate the test coverage, we recommend using the cargo tarpaulin command:\ncargo tarpaulin --out Html --output-dir ./tarpaulin-report\n\nRunning this command generates an HTML file that provides detailed coverage information for all packages. Specifically, it highlights the orml-currencies-allowance-extension pallet, which currently has a coverage of only 22.22%. Additionally, it reveals the absence of tests for the methods utilized in the runtime to implement ChainExtension, including is_allowed_currency, allowance, do_approve_transfer, and do_transfer_approved, as well as the lack of testing for the overall ChainExtension implementation in the foucoco runtime.\n\nRecommendation:\nTo address the low test coverage in the orml-currencies-allowance-extension pallet and the lack of test coverage in the runtime implementation of ChainExtension, it is essential to develop a comprehensive test suite. This suite should include thorough testing to ensure the security, stability, and maintainability of the project.\n\nImplementing continuous integration (CI) systems to automate the execution of the test suite is highly recommended. This practice enables the team to identify areas with insufficient test coverage, detect regressions, and uncover areas that require improvement. By incorporating CI into the development workflow, valuable feedback is obtained, ultimately enhancing the overall quality of the codebase.",
"description_summary": "Insufficient test coverage in key areas affects security, stability, and maintainability."
},
{
"title": "Vulnerable and Unmaintained Dependencies",
Expand Down
Loading

0 comments on commit 9203735

Please sign in to comment.