fix: 🛠️ Sherlock audit remediation by timbrinded · Pull Request #117 · chain-ml/tq-oracle

timbrinded · 2025-11-21T19:12:35Z

Warning

This should not be attempted to be merged until #130 updates this branch with the latest fixes.

Contains remediation for all of the issues raised by the Sherlock Auditor.

…/add-stakewise

…audit-issues-sherlock

* fix: 🐛 fix cowswap scaling * refactor: ♻️ Tidy code

* fix: 🔧 add timeouts added timeout to safe external api so it doesn't hang indefinitely * feat: 🩹 add cowswap timeout * increase timeout --------- Co-authored-by: timbrinded <79199034+timbrinded@users.noreply.github.com>

* Add Pyth API response validation with JSON and structure checks * Properly validate feed items are dicts instead of silently filtering * Refactor tests to reduce code duplication using helper functions * chore: fmt * Fix tests to use monkeypatch instead of mocker fixture * Apply ruff formatting * switch to pydantic model * fmt --------- Co-authored-by: timbrinded <79199034+timbrinded@users.noreply.github.com>

…ythAdapter (#107) Co-authored-by: matias-gonz <maigonzalez@fi.uba.ar>

* refactor: 🛠️ Change fetch_native_price return type to string to prevent float precision loss * add test * Add CowSwap API response validation with timeout and structure checks * chore: lint * Remove redundant numeric price and timeout tests * fix tests --------- Co-authored-by: timbrinded <79199034+timbrinded@users.noreply.github.com>

Copilot

Pull request overview

This PR addresses audit issues for the Sherlock oracle by improving robustness, error handling, and test coverage. The changes focus on fixing precision issues in price calculations, enhancing retry logic, adding comprehensive input validation, and removing unused code.

Removes unused code (units.py, safe_state.py adapter) and adds test coverage for pricing, validation, and timeout behavior
Introduces global pipeline timeout, price validation retry logic with backoff, and HTTP request retry mechanisms
Fixes decimal precision handling in price adapters (Pyth and CowSwap) to properly account for token decimals

Reviewed changes

Copilot reviewed 30 out of 31 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`uv.lock`, `pyproject.toml`	Adds pytest-mock dependency for improved testing
`tq-oracle.toml.example`	Adds configuration examples for new timeout and Pyth settings
`src/tq_oracle/settings.py`	Adds global timeout, price validation retry settings, and Pyth fail flags
`src/tq_oracle/main.py`	Adds CLI option for global timeout
`src/tq_oracle/pipeline/run.py`	Implements global timeout wrapper, removes unused address canonicalization
`src/tq_oracle/pipeline/pricing.py`	Adds retry logic with backoff for price validation
`src/tq_oracle/pipeline/assets.py`	Adds block_identifier to subvault queries
`src/tq_oracle/report/publisher.py`	Adds retry logic for Safe API calls
`src/tq_oracle/adapters/price_adapters/pyth.py`	Adds decimal-aware price calculations, input validation, and stale/missing feed tracking
`src/tq_oracle/adapters/price_adapters/cow_swap.py`	Removes unused get_token_decimals, improves error handling and precision
`src/tq_oracle/adapters/price_adapters/eth.py`	Removes redundant validate_prices call
`src/tq_oracle/adapters/price_validators/pyth.py`	Adds fail-on-stale and fail-on-missing price logic
`src/tq_oracle/adapters/asset_adapters/idle_balances.py`	Optimizes by caching supported assets query
`src/tq_oracle/processors/total_assets.py`	Refactors division to avoid precision loss
`src/tq_oracle/processors/oracle_helper.py`	Improves error handling for empty vaults
`src/tq_oracle/checks/price_validators.py`	Adds retry_recommended flag to PriceValidationError
`src/tq_oracle/adapters/check_adapters/timeout_check.py`	Adds check for suspicious reports allowing immediate replacement
`src/tq_oracle/adapters/check_adapters/safe_state.py`	Removes unused Safe state check adapter
`src/tq_oracle/adapters/check_adapters/__init__.py`	Removes SafeStateAdapter from check list
`src/tq_oracle/abi.py`	Updates get_oracle_address_from_vault to use settings object and block_identifier
`tests/test_units.py`	Removes tests for deleted scale_to_18 function
`tests/pipeline/test_run_timeout.py`	Adds tests for global timeout behavior
`tests/checks/test_price_validations.py`	Adds tests for price validation retry logic
`tests/adapters/price_validators/test_pyth.py`	Updates tests for decimal-aware pricing and adds stale/missing feed tests
`tests/adapters/price_adapters/test_pyth.py`	Adds comprehensive error handling tests
`tests/adapters/price_adapters/test_cow_swap.py`	Adds precision and error handling tests
`tests/adapters/asset_adapters/test_idle_balances.py`	Updates test to match supported_assets caching
`README.md`	Documents new global_timeout_seconds option

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-21T19:15:27Z

src/tq_oracle/adapters/price_adapters/cow_swap.py

+            price_decimal = Decimal(str(price_value))
+        except Exception as e:


Catching bare Exception is too broad. Specify the expected exception types (e.g., ValueError, TypeError, decimal.InvalidOperation) to avoid masking unexpected errors.

Copilot · 2025-11-21T19:15:27Z

src/tq_oracle/pipeline/run.py

+        )
+        raise asyncio.TimeoutError(
+            "Report exceeded global timeout "
+            f"{timeout_s}s (vault={vault_address})\n N.B. This can be changed via `global_timeout_seconds` "


The error message contains a bare newline followed by 'N.B.' which may not display properly in all contexts. Consider using a space or reformatting to: f\"{timeout_s}s (vault={vault_address}). This can be changed via 'global_timeout_seconds' or CLI flag '--global-timeout-seconds'.\"

Suggested change

f"{timeout_s}s (vault={vault_address})\n N.B. This can be changed via `global_timeout_seconds` "

f"{timeout_s}s (vault={vault_address}). N.B. This can be changed via `global_timeout_seconds` "

Copilot · 2025-11-21T19:15:28Z

tests/adapters/price_validators/test_pyth.py

    assert validator.failure_tolerance == 1.0


+async def _usdc_decimals(self, _token_address: str) -> int:


The helper function _usdc_decimals is defined at module level (outside any test function or class) but takes self as a parameter. This suggests it's meant to be a method, but it's not part of any class. Either remove the self parameter or move it inside the appropriate test class/fixture.

Suggested change

async def _usdc_decimals(self, _token_address: str) -> int:

async def _usdc_decimals(_token_address: str) -> int:

Copilot · 2025-11-21T19:15:28Z

src/tq_oracle/processors/oracle_helper.py

+        logger.error("OracleHelper contract call failed: %s", str(e))
+
+        if config.ignore_empty_vault and total_assets == 0:


The error is logged before checking conditions, meaning the error will always be logged even when it's expected (empty vault with ignore flag). Consider moving the error logging inside the else block at line 81 where the exception is re-raised, or making it a warning when the vault is empty.

Copilot · 2025-11-21T19:15:28Z

src/tq_oracle/adapters/price_adapters/pyth.py

+            decimals = await self.get_token_decimals(original_address)
+            price_per_base_unit_d18 = (asset_price_18 * 10**18) // (10**decimals)


The get_token_decimals method is called for every asset inside a loop without caching. This could result in multiple RPC calls for the same token. Consider adding a decimals cache similar to what was removed from CowSwapAdapter, or calling this once per unique asset before the loop.

Copilot · 2025-11-21T19:15:28Z

tests/adapters/price_validators/test_pyth.py

        # Oracle price: USDC is ~1/3000 ETH (since ETH is $3000 and USDC is $1)
+        monkeypatch.setattr(
+            "tq_oracle.adapters.price_adapters.pyth.PythAdapter.get_token_decimals",
+            _usdc_decimals,


The _usdc_decimals function is defined with a self parameter but is being used as a standalone function via monkeypatch.setattr. This will cause a TypeError when called because the function expects self but won't receive it in this patching context. Either remove self from the function signature or use a lambda/partial to properly bind it.

Suggested change

_usdc_decimals,

lambda *args, **kwargs: _usdc_decimals(),

* fix dps * fix tests

* fix dps * fix tests * fix log

Copilot

Pull request overview

Copilot reviewed 35 out of 36 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-23T11:53:47Z

src/tq_oracle/processors/total_assets.py

@@ -1,7 +1,10 @@
 from __future__ import annotations

+from _decimal import ROUND_DOWN


Import ROUND_DOWN from decimal instead of _decimal. The _decimal module is an internal implementation detail. Use from decimal import Decimal, ROUND_DOWN for consistency with the rest of the codebase (see oracle_helper.py line 4).

Suggested change

from _decimal import ROUND_DOWN

from decimal import ROUND_DOWN

Copilot · 2025-11-23T11:53:48Z

src/tq_oracle/processors/total_assets.py

+from _decimal import ROUND_DOWN
+
 from ..adapters.price_adapters.base import PriceData
 from ..processors.asset_aggregator import AggregatedAssets
+from decimal import Decimal




Group imports from the same module together. Both Decimal and ROUND_DOWN should be imported on the same line: from decimal import Decimal, ROUND_DOWN. This matches the pattern used in oracle_helper.py (line 4).

Suggested change

from _decimal import ROUND_DOWN

from ..adapters.price_adapters.base import PriceData

from ..processors.asset_aggregator import AggregatedAssets

from decimal import Decimal

from decimal import Decimal, ROUND_DOWN

from ..adapters.price_adapters.base import PriceData

from ..processors.asset_aggregator import AggregatedAssets

* fix dps * fix tests * fix log * fix dp rounding

fyeo-audit · 2026-01-15T13:51:34Z

✅ 3rd-Party Security Audit - `e5f5ce6`

Commit: e5f5ce6ae6db627e744033dcdd150ae4bd7e2e26
Branch: fix/audit-issues-sherlock → master
Status: No Security Issues Found
Risk Score: 3.2/10 (Low)

Security Audit Report

Generated: 2026-01-15 13:51:34
Primary Package: tq-oracle v0.1.0
Chains Analyzed: 3

Supply Chain Risk Dashboard

🟢 Risk Score: 3.2/10 (Low)

Quick Stats

Metric	Value
Dependencies Analyzed	9
Security Findings	0 (0 critical, 0 high, 0 medium)
Abandoned Packages	1
Stale Packages	0
Packages with Advisories	2 (total: 9)
Packages with Updates	7

Top Actions

🟠 [HIGH] Replace abandoned package backoff (unmaintained)
🟠 [HIGH] Review advisories for pydantic (2 advisory)
🟠 [HIGH] Review advisories for requests (7 advisory)

Executive Summary

Analyzed 3 unique external functions (from 3 call chains). Found 0 confirmed security issues.

Findings Summary

Severity	Count
CRITICAL	0
HIGH	0
MEDIUM	0
LOW	0

Package Dependencies Analyzed

Package	Version	Functions Analyzed	Repository
`typer`	`0.19.2`	3	N/A

Dependency Health Summary

Package	Version	Status	Last Update	Advisories
`backoff`	`2.2.1` (latest)	🔴 Abandoned	1197d ago	✓ None
`pydantic`	`2.6` → `2.12.5` ⬆️	🟢 Active	49d ago	🚨 2 issues
`pydantic-settings`	`2.2` → `2.12.0` ⬆️	🟢 Active	65d ago	✓ None
`python-dotenv`	`1.1.1` → `1.2.1` ⬆️	🟢 Active	80d ago	✓ None
`requests`	`2.32.5` (latest)	🟢 Active	149d ago	🚨 7 issues
`safe-eth-py`	`7.13.0` → `7.18.0` ⬆️	🟢 Active	27d ago	✓ None
`tomli`	`2.0.0` → `2.4.0` ⬆️	🟢 Active	4d ago	✓ None
`typer`	`0.19.2` → `0.21.1` ⬆️	🟢 Active	9d ago	✓ None
`web3`	`7.13.0` → `7.14.0` ⬆️	🟢 Active	90d ago	✓ None

Security Advisories

`pydantic` (v2.6)

🟡 MEDIUM (CVSS 5.9) ✅ Not affected
- Pydantic regular expression denial of service
- CVE: CVE-2024-3772
- Vulnerable versions: >= 2.0.0, < 2.4.0, < 1.10.13
🟡 MEDIUM (CVSS 3.3) ✅ Not affected
- Use of "infinity" as an input to datetime and date fields causes infinite loop in pydantic
- CVE: CVE-2021-29510
- Vulnerable versions: < 1.6.2, >= 1.8, < 1.8.2, >= 1.7, < 1.7.4

`requests` (v2.32.5)

🟠 HIGH (CVSS 7.5) ✅ Not affected
- Insufficiently Protected Credentials in Requests
- CVE: CVE-2018-18074
- Vulnerable versions: <= 2.19.1
🟡 MEDIUM (CVSS 5.3) ✅ Not affected
- Requests vulnerable to .netrc credentials leak via malicious URLs
- CVE: CVE-2024-47081
- Vulnerable versions: < 2.32.4
🟡 MEDIUM (CVSS 5.6) ✅ Not affected
- Requests Session object does not verify requests after making first request with verify=False
- CVE: CVE-2024-35195
- Vulnerable versions: < 2.32.0
🟡 MEDIUM (CVSS 6.1) ✅ Not affected
- Unintended leak of Proxy-Authorization header in requests
- CVE: CVE-2023-32681
- Vulnerable versions: >= 2.3.0, < 2.31.0
🟡 MEDIUM (CVSS 5.3) ✅ Not affected
- Exposure of Sensitive Information to an Unauthorized Actor in Requests
- CVE: CVE-2014-1829
- Vulnerable versions: < 2.3.0
🟡 MEDIUM ✅ Not affected
- Exposure of Sensitive Information to an Unauthorized Actor in Requests
- CVE: CVE-2014-1830
- Vulnerable versions: < 2.3.0
🟡 MEDIUM ✅ Not affected
- Python Requests Session Fixation
- CVE: CVE-2015-2296
- Vulnerable versions: >= 2.1.0, < 2.6.0

Detailed Findings

Analyzed Chains - No Confirmed Issues (3)

The following 3 chains were analyzed and passed peer review with no confirmed security issues.

Package	Functions Analyzed	Total Call Paths
`typer`	3	3

View all 3 analyzed chains

1. src.tq_oracle.main::report → typer.Exit

Package: typer v0.19.2

2. src.tq_oracle.main::report → typer.echo

Package: typer v0.19.2

3. src.tq_oracle.main::report → typer.BadParameter

Package: typer v0.19.2

Analysis Metadata

Total Analysis Time: 23.4 seconds
Average Time per Chain: 7.8 seconds
Generated At: 2026-01-15T13:51:34.087793

Generated with 3rd-Party Library Scanner

Generated by 3rd-Party Library Scanner

timbrinded added 30 commits November 4, 2025 20:37

feat: ✨ initial attempt

78e1a56

Merge branch 'master' of github.com:Moonsong-Labs/tq-oracle into feat…

02eac5d

…/add-stakewise

Fix Tests

479a013

fixed stakewise support

5c0bcc5

minor progress

3bf8640

first attempt

5ff65e5

tidy

c8b9f02

tidy

296d558

refactor out leveraged logic

3f2bfe9

tidy

430513f

new options

58d8a35

config schema change

74263d1

tidy

df1607f

update deps

5bf20a9

pr comment fixes

f0eff94

tidy

6ca299d

tidy assets

daf1a79

Merge branch 'master' of github.com:Moonsong-Labs/tq-oracle into feat…

435eb60

…/add-stakewise

tidy

a63d765

Merge branch 'master' of github.com:Moonsong-Labs/tq-oracle into fix/…

982de6b

…audit-issues-sherlock

refactor: ♻️ Tidy pointless check (#111)

3b3fd56

feat: ✨ Add pyth failure toggles (#112)

d9de131

fix: 🐛 Make block number consistent across all RPC calls (#113)

13d049c

feat: ✨ add global timeout (#116)

93314e6

make failed confidence checks raise errors (#115)

dac25d2

chore: rm unused scale fn (#114)

f6b10ed

feat: ⚡ added backoff retries (#110)

e4e0865

fix: 🔧 tighten empty vault error handling (#106)

054285a

feat: ✨ Add post check retry (#105)

7b27b98

fix: 🐛 use proper timestamp for timeout check (#103)

db792c1

timbrinded and others added 9 commits November 21, 2025 16:46

fix: 🐛 collect dust (#97)

682b41a

refactor: ♻️ tidy code (#96)

e3b0ecb

fix: 🛠️ Correct price scaling across adapters (#108)

3ce7240

* fix: 🐛 fix cowswap scaling * refactor: ♻️ Tidy code

refactor: ♻️ tidy code (#104)

05044d5

fix: add missing timeouts to HTTP requests (#101)

234f883

* fix: 🔧 add timeouts added timeout to safe external api so it doesn't hang indefinitely * feat: 🩹 add cowswap timeout * increase timeout --------- Co-authored-by: timbrinded <79199034+timbrinded@users.noreply.github.com>

refactor: ♻️ tidy code (#95)

2bb0d44

feat: confidence check fails if confidence ratio exceeds maximum in P…

2e2fd9d

…ythAdapter (#107) Co-authored-by: matias-gonz <maigonzalez@fi.uba.ar>

Copilot AI review requested due to automatic review settings November 21, 2025 19:12

Copilot started reviewing on behalf of timbrinded November 21, 2025 19:13 View session

Copilot finished reviewing on behalf of timbrinded November 21, 2025 19:14

Copilot AI reviewed Nov 21, 2025

View reviewed changes

timbrinded added 2 commits November 23, 2025 11:36

Fix/final report dps (#119)

eb9b801

* fix dps * fix tests

Fix/final report dps (#120)

2bb4bfe

* fix dps * fix tests * fix log

Copilot AI review requested due to automatic review settings November 23, 2025 11:51

Copilot started reviewing on behalf of timbrinded November 23, 2025 11:52 View session

Copilot finished reviewing on behalf of timbrinded November 23, 2025 11:53

Copilot AI reviewed Nov 23, 2025

View reviewed changes

Fix/final report dps (#121)

e5f5ce6

* fix dps * fix tests * fix log * fix dp rounding

timbrinded added enhancement New feature or request FYEO FYEO auditor changes labels Dec 2, 2025

timbrinded closed this Dec 2, 2025

timbrinded reopened this Dec 2, 2025

timbrinded added sherlock Sherlock auditor changes audit and removed FYEO FYEO auditor changes enhancement New feature or request labels Dec 2, 2025

timbrinded changed the title ~~Fix/audit issues sherlock~~ fix: 🛠️ Sherlock audit remediation Dec 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: 🛠️ Sherlock audit remediation#117

fix: 🛠️ Sherlock audit remediation#117
timbrinded wants to merge 44 commits intomasterfrom
fix/audit-issues-sherlock

timbrinded commented Nov 21, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

Copilot AI Nov 23, 2025

Uh oh!

fyeo-audit commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		price_decimal = Decimal(str(price_value))
		except Exception as e:

	f"{timeout_s}s (vault={vault_address})\n N.B. This can be changed via `global_timeout_seconds` "
	f"{timeout_s}s (vault={vault_address}). N.B. This can be changed via `global_timeout_seconds` "

		assert validator.failure_tolerance == 1.0


		async def _usdc_decimals(self, _token_address: str) -> int:

	async def _usdc_decimals(self, _token_address: str) -> int:
	async def _usdc_decimals(_token_address: str) -> int:

		logger.error("OracleHelper contract call failed: %s", str(e))

		if config.ignore_empty_vault and total_assets == 0:

		decimals = await self.get_token_decimals(original_address)
		price_per_base_unit_d18 = (asset_price_18 * 1018) // (10decimals)

		@@ -1,7 +1,10 @@
		from __future__ import annotations

		from _decimal import ROUND_DOWN

	from _decimal import ROUND_DOWN
	from decimal import ROUND_DOWN

Conversation

timbrinded commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

Uh oh!

fyeo-audit commented Jan 15, 2026

✅ 3rd-Party Security Audit - e5f5ce6

Security Audit Report

Supply Chain Risk Dashboard

Quick Stats

Top Actions

Executive Summary

Findings Summary

Package Dependencies Analyzed

Dependency Health Summary

Security Advisories

pydantic (v2.6)

requests (v2.32.5)

Detailed Findings

Analyzed Chains - No Confirmed Issues (3)

Analysis Metadata

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

timbrinded commented Nov 21, 2025 •

edited

Loading

✅ 3rd-Party Security Audit - `e5f5ce6`

`pydantic` (v2.6)

`requests` (v2.32.5)