Home Original page

feat(net): verify columns' length of HelloMessage by 317787106 · Pull Request #5667 · tronprotocol/java-tron

@317787106

What does this PR do?

  • verify columns of hellomessage, including address, sig & codeVersio, length should not be over 200

Why are these changes required?

  • avoid too long log for HelloMessage

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

@317787106

jwrct

return false;
}

int maxByteSize = 200;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a maximum byte size of 200 too large? Can it be set smaller?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The target of method valid is to make sure that log's content is not too large, so the value of maxByteSize is not very important. The length of address may be 42 bytes or more (not certain), sig is 65, codeVersion may not stable(20 bytes?). Can you give a suggestion?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just take the maximum of the three values, for example 65.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, I don't test the true length of address ang sig, because i have no fastforword node to use. Only give a theoretical value. Give an upper value can be ok, but give an exact value is not necessary, because method checkHelloMessage will verify the address and sig again. The main purpose is to not record too many message in log file.

xxo1shine

if (!codeVersion.isEmpty() && codeVersion.toByteArray().length > maxByteSize) {
return false;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The length judgment of address and version can be more specific.
It is recommended to consider multi-signature within 3 keys, If there are 3 multi-signatures, will the length exceed 200?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this signature only come from SR's sig of timestamp in RelayService. it's 65 stablely, not related to multi-signature. You can read the code:

      String sig =
          TransactionCapsule.getBase64FromByteString(msg.getSignature());
      byte[] sigAddress = SignUtils.signatureToAddress(hash.getBytes(), sig,
          Args.getInstance().isECKeyCryptoEngine());

jwrct

@jwrct jwrct changed the title feat(log): verify columns' length of HelloMessage feat(net): verify columns' length of HelloMessage

Mar 12, 2024