Lightlink Bridge: BlockApex WhiteBox Code Review Report

Table Of Content

Share:

Executive Summary

Report Objectives

This document details the source code review of Lightlink Bridge Validator and Keeper. The purpose of the assessment was to perform the whitebox testing of the Bridge’s validator and Keeper before going into production and identify potential threats and vulnerabilities.

Scope of Work

TypeDetails
Target System NameLightLink ValidatorLightLink Keeper
Source Code URL(s)/ https://github.com/pellartech/ll-bridge-validator/commit/34977df32ecd8079701181cb632d4dbb2b119aac
https://github.com/pellartech/ll-bridge-keeper/commit/17a7d47e8b8473d8e34b44681f8a0d35bdb6aa47
Type of TestWhite-Box Testing
LanguageTypeScript

Project Objectives

Conduct a thorough code-review and vulnerability research on Lightlink Bridge’s Validator and keeper code.

Results

The codebase is written in good quality. No significant issue was identified which can affect the functionality of the bridge. Error handling should be improved. For overall understanding of the codebase, proper commenting should be applied.

Summary of Findings

Our Team Found

# of issuesSeverity Of the Risk
0Critical Risk Issues(s)
1High Risk Issues(s)
2Medium Risk Issues(s)
1Low Risk Issues(s)
0Informatory Risk Issues(s)
Lightlink Bridge - pie chart

Methodology

Planning

Our general approach during this code audit was as follows:

Code review

The first step of our audit was a thorough code review. We read and go through the code, ensuring a comprehensive understanding of the bridge's architecture not limited to the superficial aspects of the code.

Static analysis 

The next stage in our audit involved conducting a rigorous static analysis using SonarQube. This powerful tool allowed us to analyze the codebase in a non-runtime environment. With SonarQube, we were able to detect & verify bugs, vulnerabilities, and code smells to mitigate any risks and improve overall code quality.

Dynamic Analysis

Our final audit stage consisted of dynamic analysis. This process involved integrating test functions within the code and running the program with a range of inputs. Our goal was to trigger and observe any suspicious or unexpected behaviors that might emerge during runtime. This approach helped us to identify potential issues that only manifest themselves under specific conditions or when the system is operational.

Risk Classification

This report is adhering to the classification standards defined as per OWASP. The summarized version is presented below.

LightLink Bridge Architecture

 System Architecture

The LightLink Bridge system architecture is based on a Proof-of-Authority mechanism with an external validator set that stakes its individual reputation in order to earn incentives for the trades on the LightLink bridge. This ensures a robust and interconnected framework enabling seamless asset transfers between Ethereum's Layer 1 and Layer 2. The bridge architecture consists of several key components, each playing a crucial role in the overall operation of the bridge. The bridge architecture consists of three main components, the front-end dApp, validator nodes and a single keeper node.

Architecture Diagram

Note:

While the provided documentation for the LightLink architecture is commendable and provides an essential overview, it could benefit from further detailed elaboration. A comprehensive breakdown of each component's functionality, interactions, and the specific roles they play in the broader architecture could enhance the depth and clarity of the document. Additionally, providing more detailed technical insights, workflows, and possible edge cases could facilitate a more robust understanding of the system. This will not only aid in improved transparency but also contribute to more effective and inclusive future audits, and security assessments.

Workflow of Keeper and Bridge

Findings

Potential Risks of Unauthorized Access on Redis

SeverityLikelihoodAffected Files
HighMedium  src/config/environments/production.ts

Description:

Redis is used by both Validator and Keeper. Under current implementation of the validator and keeper code, there was no authentication applied for redis connection. If the same code is deployed on production without using proper authentication mechanism, it can lead to potential RCE on the redis server. Open redis servers are prone to Remote Code Execution issues.

export default class RedisProvider {
@inject(LLogger.name) private _logger: LLogger

protected redis: RedisClientType
public readonly REDIS_PORT = CONFIG.REDIS.PORT
public readonly REDIS_HOST = CONFIG.REDIS.HOST
public readonly REDIS_PREFIX = CONFIG.REDIS.PREFIX

constructor(@inject(LLogger.name) logger: LLogger) {
this._logger = logger
this.redis = createClient({
url: `redis://${CONFIG.REDIS.HOST}:${CONFIG.REDIS.PORT}`
})

this.redis.on('error', err => {
this.disconnect()
this._logger.info(`Redis Client Error: ${err}`)
})
this.connect().then(() => {
this._logger.info('Redis Client Connected')
})
}

/ll-bridge-keeper-17a7d47e8b8473d8e34b44681f8a0d35bdb6aa47/src/providers/redis.ts

Proof Of Concept (POC)

File: “ll-bridge-keeper-17a7d47e8b8473d8e34b44681f8a0d35bdb6aa47/src/config/environments/production.ts”

REDIS: {
HOST: String(process.env.REDIS_HOST || '127.0.0.1'),
PORT: Number(process.env.REDIS_PORT || 6379),
PREFIX: String(process.env.REDIS_PREFIX || '__ll_bridge_keeper__')
},

Mitigation:

The production code must incorporate the use of a Redis instance with the appropriate credentials in order to address potential unauthorized access to Redis.

Developer Response:

All validators are private and run via a docker package prepared by the development team. There is no way to access the packages in an unauthorized manner and all validators may only be run via a whitelisting process via the distributed docker solution.

Redis also does not store any security data.

Auditor Response:

The response from developers is acceptable and acknowledged. Open Redis instance are prone to RCE issues. We reported the issue in that sense. It will be a good practice to use credentials on redis too.

Potential DOS Risk due to whitelisted validators

SeverityLikelihoodAffected Files
MediumLowsrc/modules/v1/chain/chain.indexer.ts

Description:

A temporary DOS can happen if following scenarios are carried out

  1. User submits a transaction for bridging
  2. Validator submits a pending proof in DB
  3. Validators are updated on chain by admin
  4. Keeper will submit transaction on chain but as validators are updated hence validation will not be successful
  5. Keeper will repeatedly send request until it is successful
  6. If there are 10 such proofs in DB then it can cause temporary DOS as in the produceValidatedProofs() method , getVerifiedProofs() is taking a limit of 10 , and it will fetch 10 such proofs which can cause issues.
public async produceValidatedProofs() {
const requests = await Promise.all([
this._v1BridgeRepository.getVerifiedProofs({
chain: ChainSupported.Lightlink,
consensusPowerThreshold: CONFIG.MODULES.V1.BRIDGE.CONTRACTS.LIGHTLINK.CONSENSUS_POWER_THRESHOLD,
limit: 10
})
// this._v1BridgeRepository.getVerifiedProofs({
// chain: ChainSupported.Ethereum,
// consensusPowerThreshold: CONFIG.MODULES.V1.BRIDGE.CONTRACTS.ETHEREUM.CONSENSUS_POWER_THRESHOLD,
// limit: 10 // })
])
// const items = [...requests[0], ...requests[1]]
const items = [...requests[0]]

for (const item of items) {
await this.submitProofToChain(item)
}}

src/modules/v1/chain/chain.service.ts

Mitigation:

It is important to include a mechanism that ensures when onchain validators are updated, the corresponding whitelisted validators should also be updated to prevent any potential scenarios.

Developer Response:

Configuration operating with validators is as follows:

1 x Validator with 40 voting power run by Pellar

2 x Validator with 20 voting power run by Pellar

Voting power required for authorisation of bridge transfers = 80

At the moment in the unlikely case that a validator is removed from the pool then we can monitor any proofs that are invalid due to this circumstance and process the withdrawal.

To mitigate the chance of us not being able to reach 80 voting power we are onboarding partners who will be running validators each with 20 power of their own.

Auditor Response:
The response from developers is acceptable and acknowledged.

Potential Risk of Price Manipulation Due to single source

SeverityLikelihoodAffected Files
MediumHighsrc/config/environments/production.ts 

Description:

Keeper is fetching prices from a single oracle source i.e gate.io , which is defined in src/config/environments/production.ts

EXTERNAL_PARTIES: {
GATE_IO: {
API_ROOT_V4: 'https://api.gateio.ws/api/v4'
}}}

In case of a price manipulation on gate.io , bridge can be affected.

Mitigation:

It is advised to define multiple price oracles and aggregate the price or use Chainlink’s price oracle as it aggregate price from multiple sources.

Developer Response:
We are not currently fetching prices from any oracle so this is a non-issue. If/when we do then we will make sure to either use an aggregated source such as Chainlink or aggregate sources on our own.

Auditor Response:

The response from developers is acceptable and acknowledged.

Information Disclosure due to Improper Error Handling

SeverityLikelihoodAffected Files
LowHighsrc/core/apiError.ts

Description:

When a Keeper node is deployed it deploys certain API endpoints to which the validator can see the proofhashes & signatures. If the data is not sent in proper format then the endpoint returns the full stack error. This stack shows the full path of the deployed keeper, which is a bad practice & leaks information.

Technical Description:

In ​​’src/core/apiError.ts’ if the type is ‘RequestException’ then it is returning exceptionInstance.resolve(). resolve() method is also returning the “this.stack”.

export default class ErrorFactory {
static handle(type: Keys, error: IException, context?: IExceptionContext): IException {
const exceptionInstance = new exceptionMaps[type](error, context)
switch (type) {
case 'BizException':
case 'RequestException':
return exceptionInstance.resolve()
}
}
public resolve() {
return {
status: this.status,
code: this.code,
message: this.message,
details: this.details,
context: this.context,
stack: this.stack
}}}

Proof Of Concept (POC)

  1. Make a bad parameterized request at the /api/v1/bridges/finalized/proofs
  2. Analyze the response

Mitigation:

It’s advised not to return the stack and detailed messages, rather it is recommended to only return generic error codes upon a bad request. This obscurity prevents attackers from gaining insights into the backend infrastructure from the error information, thereby limiting their scope for black box exploitation. It mitigates the potential reconnaissance performed by malicious actors.

Developer Response:
Access to the stack has been removed in production mode.

Auditor Response:
The response from developers is acceptable and acknowledged.

SonarQube Test Reports

We utilized SonarQube, a powerful tool designed to inspect and analyze the entire codebase for Keeper and Validator. The primary purpose of SonarQube is to conduct static code analysis, systematically examining the software without executing it, to identify potential vulnerabilities, bugs, and code smells. Code smells refer to certain characteristics or patterns in the code that could potentially indicate deeper problems. SonarQube provided us with several insightful suggestions for enhancing our code's quality and security. The following points highlight these suggested improvements and identified issues:

SonarQube Issues Report for Keeper

RuleSeverityFileLineDescription
typescript:S4144MAJORkeeper:src/core/apiError.ts24Update this function so that its implementation is not identical to the one on line 15.
typescript:S4323MINORkeeper:src/helpers/utils.ts4Replace this union type with a type alias.
typescript:S4323MINORkeeper:src/modules/v1/bridge/bridge.repository.ts38Replace this union type with a type alias.
typescript:S1135INFOkeeper:src/modules/v1/chain/chain.service.ts537Complete the task associated to this "TODO" comment.
typescript:S4822MAJORkeeper:src/modules/v1/system/system.indexer.ts
25Consider using 'await' for the promise inside this 'try' or replace it with 'Promise.prototype.catch(...)' usage.
typescript:S4822MAJORkeeper:src/modules/v1/system/system.indexer.ts37Consider using 'await' for the promise inside this 'try' or replace it with 'Promise.prototype.catch(...)' usage.
typescript:S4325MINORkeeper:src/providers/external.ts71This assertion is unnecessary since it does not change the type of the expression.

SonarQube Issues Report For Validator

typescript:S1135INFOvalidator:src/modules/v1/bridge/bridge.service.ts213Complete the task associated to this "TODO" comment.
typescript:S3776CRITICALvalidator:src/modules/v1/chain/chain.service.ts35Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
typescript:S4323MINORvalidator:src/modules/v1/chain/chain.service.ts55Replace this union type with a type alias.
typescript:S3776CRITICALvalidator:src/modules/v1/chain/chain.service.ts206Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
typescript:S3776CRITICALvalidator:src/modules/v1/chain/chain.service.ts379Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
typescript:S3776CRITICALvalidator:src/modules/v1/chain/chain.service.ts556Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
typescript:S3776CRITICALvalidator:src/modules/v1/chain/chain.service.ts730Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
typescript:S3776CRITICALvalidator:src/modules/v1/chain/chain.service.ts884Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
typescript:S3776CRITICALvalidator:src/modules/v1/chain/chain.service.ts1038Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
typescript:S3776CRITICALvalidator:src/modules/v1/chain/chain.service.ts1197Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.
typescript:S4822MAJORvalidator:src/modules/v1/system/system.indexer.ts18Consider using 'await' for the promise inside this 'try' or replace it with 'Promise.prototype.catch(...)' usage.

NPM Audit Report

Npm offers a dependency auditing option which should be run by the developer to ensure usage of secure dependencies and to mitigate the risk of supply chain attacks.

Vulnerable
Dependency
TitleSeverityViaEffectsRange
word-wrapword-wrap vulnerable to Regular Expression Denial of Servicemoderateword-wrap-<1.2.4
semversemver vulnerable to Regular Expression Denial of Servicemoderatesimple-update-notifier-<=5.7.1
nodemon-moderatesimple-update-notifier-2.0.19 - 2.0.22
tough-cookietough-cookie Prototype Pollution vulnerabilitymoderatetough-cookie-<4.1.3
minimistPrototype Pollution in minimistcritical-optimist<=0.2.3
mongooseMongoose Prototype Pollution vulnerabilitycritical--6.0.0 - 6.11.2
optimistPrototype Pollution in minimistcriticalminimistswig-templates>=0.6.0
swig-templatesArbitrary local file read vulnerability during template renderingcriticalswig-templates, optimistswig-email-templates*
class-validatorSQL Injection and Cross-site Scripting in class-validatorcritical--<0.14.0
@aws-sdk/client-cognito-identity-high@aws-sdk/client-sts@aws-sdk/credential-provider-cognito-identity3.12.0 - 3.54.1
@aws-sdk/client-sts-highfast-xml-parser@aws-sdk/client-cognito-identity, @aws-sdk/credential-providers<=3.54.1
cheerio-highcss-selectswig-email-templates0.19.0 - 1.0.0-rc.3
css-selectInefficient Regular Expression Complexity in nth-checkhighnth-checkcheerio<=3.1.0
fast-xml-parserfast-xml-parser vulnerable to Regex Injection via Doctype Entities, fast-xml-parser vulnerable to Prototype Pollution through tag or attribute namehighfast-xml-parser@aws-sdk/client-sts<=4.2.3
json5Prototype Pollution in JSON5 via Parse Methodhigh--<1.0.2
luxonLuxon Inefficient Regular Expression Complexity vulnerabilityhigh--1.0.0 - 1.28.0
@aws-sdk/credential-provider-cognito-identity-high@aws-sdk/client-cognito-identity-3.12.0 - 3.347.0
@aws-sdk/credential-providers-high@aws-sdk/client-cognito-identity, @aws-sdk/client-sts, @aws-sdk/credential-provider-cognito-identity-<=3.347.0
swig-email-templates-highcheerio, swig-templates->=2.0.0

Mitigation

It is advised to run ‘npm audit’ before putting it into production and using ‘npm audit fix’ to mitigate the possible issues

Disclaimer: Source Code Review

The source code review is conducted for the purpose of providing constructive feedback and suggestions to enhance the quality and security of the software. This review is not meant to criticize or undermine the efforts of the developers or individuals involved in the project. The reviewer shall not be liable for any loss, damages, or issues resulting from the use or implementation of the feedback provided. The scope of this review is limited to the specific code presented, and any subsequent changes to the code may not be reflected in this evaluation. It is essential to understand that the reviewer cannot guarantee the identification of all potential issues or vulnerabilities.

The developers are responsible for maintaining the software's security and quality. All information discovered during the review shall be treated as confidential. The use of third-party components is assumed to be compliant. The reviewer's assessment is independent, and there is no affiliation with any organization related to the software under review. The developers are encouraged to perform multiple audits, periodically reevaluate and update the code to address potential issues. By proceeding with the review results, all parties agree to accept the terms of this disclaimer.

More Audits

Infiltrating the EVM-III: Unravel the Impact Of Blockchain On Bug Fixing!

Fixing a bug in traditional software development is often likened to solving a difficult puzzle, each presenting its own challenges. This task has always been complex and time-consuming. However, resolving bugs in a blockchain system is even more demanding due to its transparent & permissionless nature and the high stakes involved with users' funds.

Revisiting Ethereum Classic in Light of the London Hard Fork

The successful upgrade of the London Hard Fork is a big difference from the fork leading to Ethereum Classic that took place back in 2016. However, despite their divergence, both are milestones in the Ethereum world- guaranteed to have lasting impacts on the blockchain as we know it. Read more to find out the circumstances surrounding each hard fork and the role they may play in shaping Ethereum's future.

Metaverse: Virtual Wonderland Or Capitalist Dystopia?

Security and privacy are among the top issues expected to arise in the metaverse. Some have even gone so far as to say that the metaverse is capitalizing on users' desire to escape from reality.

Zero-Knowledge Proofs: A Security Perspective

In a world where personal data has become more or less a commodity, the potential advantages provided by zero-knowledge proofs are monumental. By combining them with blockchain technology, a powerful mix of immutability and security can be achieved.

DeFiGeek Community JAPAN - Hack Analysis (Apr 17, 2023)

On Apr 17, 2023. The DeFiGeek Community fell victim to a security breach in which an attacker exploited a flash loan vulnerability, causing the loss of 10 ETH (valued at over $20,000) from their DeFiGeek Community Pool Dai (fDAI-102

Cast Storage

Lets understand the smart contract storage model in Ethereum and EVM-based chains and how you can access the public and private variables of any smart contract deployed on the blockchain. We can do this by using cast storage.

Unipilot Farming V2 Audit Report

BlockApex (Auditor) was contracted by  VoirStudio  (Client) for the purpose of conducting a Smart Contract Audit/ Code Review of Unipilot Farming V2. This document presents the findings of our analysis which started from  25th Feb 2022.

Vaccify - Building a Resilient Digital Trust Ecosystem

Vaccify is an open-source COVID-19 Initiative of TrustNet. The idea behind it is to issue digital certificates to people who are vaccinated (once the vaccine is available) for COVID-19. It is a Blockchain-based digital identity eco-system for all hospitals, healthcare centers, laboratories, and testing facilities across Pakistan.

Infiltrating the EVM: Advanced Strategies for Blockchain Security Guardians

Learn advanced strategies for blockchain security guardians in this groundbreaking article series by BlockApex Labs. Gain insights into the Ethereum Virtual Machine (EVM), smart contract vulnerabilities, and thorough auditing techniques. Stay ahead in the evolving world of blockchain security and prevent financial losses with comprehensive knowledge. Join us for the article series and course today.

1 2 3 11
Designed & Developed by: 
All rights reserved. Copyright 2023