Lightlink Bridge: BlockApex WhiteBox Code Review Report

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

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 issues	Severity Of the Risk
0	Critical Risk Issues(s)
1	High Risk Issues(s)
2	Medium Risk Issues(s)
1	Low Risk Issues(s)
0	Informatory Risk Issues(s)

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

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

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

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

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

SonarQube Issues Report For Validator

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.

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.