Skip to content

Conversation

@SCMusson
Copy link
Collaborator

@SCMusson SCMusson commented Apr 20, 2025

Starting on audit recommendations. The following are implemented

  • lru_cache on uplc_unflat
  • Evaluation of scripts can't use more resources than defined in protocol
  • For submit_tx calls, check requested ExUnits are valid before execution.
  • Maximum transaction size limits.

Reverted test using Redis

  • Now sharing sessions between server workers using SQLite database. Sessions are pickled and saved before each call. There is plenty of room for optimisation here but just working towards getting something functioning at the moment.
  • SlowAPI for rate limiting
  • Session lifespan / cleanup using contextlib.asynccontextmanager

Audit Recommendations

Critical

3.1.1 Memory Management in Script Evaluation

  • Unbounded Script Cache in uplc_unflat solved with lru cache. (here)
  • Unconstrained Script Size: Enforced via enforcing the maximum transaction size. (here)

3.1.2 Session Management

  • Unbounded Session Creation: SlowAPI middleware limits hourly requests by user and daily session creation rate limit
  • Session Persistence: Created a SessionManager (here) that is used by a contextlib.asynccontextmanager to periodically cleanup old sessions (here). Session lifespan is determined by environment variables MOCKFROST_MAX_IDLE_TIME, MOCKFROST_MAX_SESSION_LIFESPAN (See here)
  • State Management Vulnerabilities: There is no explicit limit on Session size, however there is an effective limit by virtue of Sessions having a limited lifespan and the number of requests being rate limited and thus there is a limit to the number of UTxOs that can be added to the internal state before the Session times out.

Medium Severity Findings

  • UTxO Set Growth: Same as above. UTxO set growth is now effectively capped by a combination of Session lifespan and rate limiting on requests. It would also be trivial to add an additional lower rate limit explicitly on requests that grow the internal state in future. @limiter.limit("3600/day") decorator on any problematic endpoints. For example this commit adds a rate limiting to submission of transactions.
  • Concurrency Issues. SESSION object is now shared between workers via a sqlite database (here, Sessions are currently just pickled and saved in a database with the default name SESSIONS.db.
  • Reference Validation.
    • I don't think UTxO existence needs to be validated as a transaction will simply fail when creating a list of input_utxos (here).
    • Ownership is currently not checked. Users are free to modify the UTxO set of a session by design. However, there are currently no witness verification during transaction evaluation other than script evaluation.
    • Potential for reference manipulation attacks. Only if you have managed to steal someone elses SESSION_ID.

Low Severity Findings

  • Exception Information Exposure. This is not a major problem
  • Inconsistent Error Handling. I have started moving towards handling errors in a similar way to Blockfrost does for consistency. for example see here and here
  • Logging Deficiencies - not critical

@coveralls
Copy link

coveralls commented Apr 20, 2025

Pull Request Test Coverage Report for Build 15742923348

Details

  • 204 of 346 (58.96%) changed or added relevant lines in 6 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-4.2%) to 81.247%

Changes Missing Coverage Covered Lines Changed/Added Lines %
plutus_bench/mockfrost/client.py 18 20 90.0%
tests/test_gift_mockfrost.py 0 2 0.0%
tests/test_mockfrost_server.py 97 102 95.1%
plutus_bench/mock.py 21 42 50.0%
plutus_bench/mockfrost/server.py 66 178 37.08%
Files with Coverage Reduction New Missed Lines %
plutus_bench/mockfrost/server.py 1 43.22%
Totals Coverage Status
Change from base Build 13635867589: -4.2%
Covered Lines: 1746
Relevant Lines: 2149

💛 - Coveralls

@socket-security
Copy link

socket-security bot commented May 21, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedpypi/​limits@​5.2.0100100100100100
Addedpypi/​deprecated@​1.2.18100100100100100

View full report

@SCMusson SCMusson marked this pull request as ready for review June 15, 2025 21:40
@SCMusson SCMusson merged commit 73dcfd9 into OpShin:main Jun 18, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants