Missing CheckMinimalPush check in MatchMultisig: largest IsStandard multisig n-of-3 is 12 bytes too large? #23285

The following four equivalent scripts all pass standardness check (IsStandard):

• 1 0x21 0x020000000000000000000000000000000000000000000000000000000000000000 1 CHECKMULTISIG
• 1 0x4c21 0x020000000000000000000000000000000000000000000000000000000000000000 1 CHECKMULTISIG
• 1 0x4d2100 0x020000000000000000000000000000000000000000000000000000000000000000 1 CHECKMULTISIG
• 1 0x4e21000000 0x020000000000000000000000000000000000000000000000000000000000000000 1 CHECKMULTISIG

Where 0x4c == OP_PUSHDATA1, 0x4d == OP_PUSHDATA2 and 0x4e == OP_PUSHDATA4.

Only the first one would pass the standardness check if minimal representation was enforced.

AFAICT we check for minimal representation in all other IsStandard standardness checks. Is there any particular reason we're not doing it for CHECKMULTISIG in MatchMultisig? :)

Good find! As far as I can see there is no reason to not enforce minimal representation also on multisig scripts to be considered as IsStandard. Happy to open a PR which fixes (and tests) this if there are no objections.

Probably want to check if some (admittedly broken) application doesn't rely on it before tightening the rules as it's still in use. This seems like a good place to start looking for such outputs.

Confirmed this is still an issue with a unit test. I can't tell if I can get the specific data needed from mainnet.observer/ but (my agents & I) will scan the chain for occurrences of non-minimal PUSHDATA in bare multisig outputs and report back...

    // Non-minimal push encodings for the pubkey (OP_PUSHDATA1/2/4) are all
    // accepted by IsStandard.  MatchMultisig uses GetOp, which decodes every
    // push form into the same raw bytes; it then calls CPubKey::ValidSize
    // (prefix byte + length, no curve check), so the encoding is invisible.
    //
    // Compressed pubkey with all-zero x: valid format for ValidSize even
    // though it is not a point on secp256k1.
    std::vector<unsigned char> pubkey_bytes(33, 0x00);
    pubkey_bytes[0] = 0x02;

    // Builds a 1-of-1 multisig with a caller-serialised pubkey push.
    const auto make_1of1 = [](const std::vector<unsigned char>& raw_push) {
        CScript s;
        s << OP_1;
        s.insert(s.end(), raw_push.begin(), raw_push.end());
        s << OP_1 << OP_CHECKMULTISIG;
        return s;
    };

    // 1. Minimal direct push: single byte 0x21 (= 33 < OP_PUSHDATA1) + 33 bytes
    {
        std::vector<unsigned char> push{0x21};
        push.insert(push.end(), pubkey_bytes.begin(), pubkey_bytes.end());
        BOOST_CHECK(is_standard(make_1of1(push)));
    }
    // 2. OP_PUSHDATA1 (0x4c): one-byte length 0x21 + 33 bytes
    {
        std::vector<unsigned char> push{OP_PUSHDATA1, 0x21};
        push.insert(push.end(), pubkey_bytes.begin(), pubkey_bytes.end());
        BOOST_CHECK(is_standard(make_1of1(push)));
    }
    // 3. OP_PUSHDATA2 (0x4d): two-byte LE length 0x2100 + 33 bytes
    {
        std::vector<unsigned char> push{OP_PUSHDATA2, 0x21, 0x00};
        push.insert(push.end(), pubkey_bytes.begin(), pubkey_bytes.end());
        BOOST_CHECK(is_standard(make_1of1(push)));
    }
    // 4. OP_PUSHDATA4 (0x4e): four-byte LE length 0x21000000 + 33 bytes
    {
        std::vector<unsigned char> push{OP_PUSHDATA4, 0x21, 0x00, 0x00, 0x00};
        push.insert(push.end(), pubkey_bytes.begin(), pubkey_bytes.end());
        BOOST_CHECK(is_standard(make_1of1(push)));
    }