Shutdown of greater than 10% or equal to but less than 30% of network processing nodes without brute force actions, but does not shut down the network
Description
Brief/Intro
Note - This is an insight report, since there is no option to submit an insight, I am submitting it under low impact, kindly downgrade to insight from low.
Active.ts::ValidateRecordTypes miss out on checking some of the types
Vulnerability Details
In the function Active.ts::ValidateRecordTypes
export function validateRecordTypes(rec: P2P.ActiveTypes.Record): string {
let err = validateTypes(rec, {
active: 'n',
activated: 'a',
activatedPublicKeys: 'a',
})
if (err) return err
for (const item of rec.activated) {
if (typeof item !== 'string') return 'items of activated array must be strings'
}
for (const item of rec.activatedPublicKeys) {
if (typeof item !== 'string') return 'items of activatedPublicKeys array must be strings'
}
return ''
}
The above function checks only for active, activated and activatedPublicKeys and their subelements (in case of arrays). The rec: P2P.ActiveTypes.Record type consists of 5 elements. These types are as follows
standby and maxSyncTime types are never checked. This can cause problems of type mismatch. The recommendation for correcting it is as follows.
Recommendation
Modify the function as follows
export function validateRecordTypes(rec: P2P.ActiveTypes.Record): string {
let err = validateTypes(rec, {
active: 'n',
@> standby: 'n',
activated: 'a',
activatedPublicKeys: 'a',
@> maxSyncTime:'n'
})
if (err) return err
for (const item of rec.activated) {
if (typeof item !== 'string') return 'items of activated array must be strings'
}
for (const item of rec.activatedPublicKeys) {
if (typeof item !== 'string') return 'items of activatedPublicKeys array must be strings'
}
return ''
}
Impact Details
It comes under security best practices of insight report
The issue is fairly simple and is insight, so I am providing only the necessary info for proof of concept.
export function validateRecordTypes(rec: P2P.ActiveTypes.Record): string {
@>let err = validateTypes(rec, { // no checking for standby and maxSyncTime
active: 'n',
activated: 'a',
activatedPublicKeys: 'a',
})
if (err) return err
for (const item of rec.activated) {
if (typeof item !== 'string') return 'items of activated array must be strings'
}
for (const item of rec.activatedPublicKeys) {
if (typeof item !== 'string') return 'items of activatedPublicKeys array must be strings'
}
return ''
}
The above function is used by various other functions such as CycleCreator.ts::validateCertsRecordTypes, so it is fairly crucial
function validateCertsRecordTypes(inp, caller) {
let err = utils.validateTypes(inp, { certs: 'a', record: 'o' })
if (err) {
warn(caller + ' bad input: ' + err + ' ' + Utils.safeStringify(inp))
return false
}
for (const cert of inp.certs) {
err = utils.validateTypes(cert, { marker: 's', score: 'n', sign: 'o' })
if (err) {
warn(caller + ' bad input.certs: ' + err)
return false
}
err = utils.validateTypes(cert.sign, { owner: 's', sig: 's' })
if (err) {
warn(caller + ' bad input.sign: ' + err)
return false
}
}
err = utils.validateTypes(inp.record, {
activated: 'a',
activatedPublicKeys: 'a',
active: 'n',
apoptosized: 'a',
counter: 'n',
desired: 'n',
duration: 'n',
expired: 'n',
joined: 'a',
joinedArchivers: 'a',
joinedConsensors: 'a',
lost: 'a',
previous: 's',
refreshedArchivers: 'a',
refreshedConsensors: 'a',
refuted: 'a',
removed: 'a',
start: 'n',
syncing: 'n',
})
if (err) {
warn(caller + ' bad input.record: ' + err)
return false
}
// submodules need to validate their part of the record
for (const submodule of submodules) {
@> err = submodule.validateRecordTypes(inp.record) // used here
if (err) {
warn(caller + ' bad input.record.* ' + err)
return false
}
}
return true
}