Bug Report (low severity)

At a high level, a malicious validator with more than 1/3 of the voting power could generate LightClientAttackEvidence that contains correct signatures but also contains bogus (e.g., unverifiable) signatures. Such LightClientAttackEvidence can be added to the evidence pool without an issue if the correct signatures stem from validators that have more than 1/3 of the voting power. As a result when we go through the signatures to slash validators we could go through bogus signatures as well and potentially slash innocent validators.

Specifically, the issue has to do in the way that LightClientAttackEvidence is verified before being added to the evidence pool. VerifyLightClientAttack in case of a lunatic attack calls VerifyCommitLightTrusting with e.ConflictingBlock.Commit. Subsequently, VerifyCommitLightTrusting calls verifyCommitBatch or verifyCommitSingle with the countAllSignatures argument set to false. This means that both verifyCommitBatch and verifyCommitSingle do not go through all the signatures contained in e.ConflictingBlock.Commit.Signatures if the signatures that have been verified so far exceed 1/3 (i.e., the default trust level) of the voting power we stop checking when the following condition evaluates to true:

if !countAllSignatures && talliedVotingPower > votingPowerNeeded {

As a result, a malicious validator (or set of validators) that has more than 1/3 of the voting power could generate LightClientAttackEvidence that has all the verifiable signatures at the beginning of the conflictingBlock.Commit.Signatures slice and afterwards it has bogus signatures that are not verifiable.
Nevertheless, in such a scenario, the LightClientAttackEvidence with the bogus signatures would still be added in the evidence pool.

When we slash based on this LightClientAttackEvidence we extract the byzantine validators that would go through all the signatures:

for _, commitSig := range l.ConflictingBlock.Commit.Signatures {
    ...
    _, val := commonVals.GetByAddress(commitSig.ValidatorAddress)

And hence we could slash validators that have their ValidatorAddress in some of the signatures but those validators have not performed a light client attack. This way, a malicious validator would get slashed, but can also succeed in slashing innocent validators.

CometBFT version: Note that the above provided links stem from CometBFT v0.38.2 but I would assume the issue exists since the introduction of LightClientAttackEvidence.

Thanks to @jmalicevic and @sergio-mena for looking into this.