descriptor_parse: crash with whitespace on pk()

[brunoerg]

The following descriptor tr(03d1d1110030000000000120000000000000000000000000001370010912cd08cc,pk( L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)) fails to be parsed on rust-miniscript (key too short) and is successfully parsed on Bitcoin Core.

There is a whitespace on pk, instead of pk(KEY) is pk( KEY) which may cause this issue.

[apoelstra]

I think this is a bug in Core.

[brunoerg]

I think this is a bug in Core.

Yes. Will check it better and open an issue there.

[brunoerg]

DecodeBase58 from Bitcoin Core skips leading spaces. The decode function fromrust-bitcoin does not.

[[nodiscard]] static bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch, int max_ret_len)
{
    // Skip leading spaces.
    while (*psz && IsSpace(*psz))
        psz++;
    // Skip and count leading '1's.
    int zeroes = 0;
    int length = 0;
    while (*psz == '1') {
        zeroes++;
        if (zeroes > max_ret_len) return false;
        psz++;
    }
    // Allocate enough space in big-endian base256 representation.
    int size = strlen(psz) * 733 /1000 + 1; // log(58) / log(256), rounded up.
    std::vector<unsigned char> b256(size);
    // Process the characters.
    static_assert(std::size(mapBase58) == 256, "mapBase58.size() should be 256"); // guarantee not out of range
    while (*psz && !IsSpace(*psz)) {
        // Decode base58 character
        int carry = mapBase58[(uint8_t)*psz];
        if (carry == -1)  // Invalid b58 character
            return false;
        int i = 0;
        for (std::vector<unsigned char>::reverse_iterator it = b256.rbegin(); (carry != 0 || i < length) && (it != b256.rend()); ++it, ++i) {
            carry += 58 * (*it);
            *it = carry % 256;
            carry /= 256;
        }
        assert(carry == 0);
        length = i;
        if (length + zeroes > max_ret_len) return false;
        psz++;
    }
    // Skip trailing spaces.
    while (IsSpace(*psz))
        psz++;
    if (*psz != 0)
        return false;
    // Skip leading zeroes in b256.
    std::vector<unsigned char>::iterator it = b256.begin() + (size - length);
    // Copy result into output vector.
    vch.reserve(zeroes + (b256.end() - it));
    vch.assign(zeroes, 0x00);
    while (it != b256.end())
        vch.push_back(*(it++));
    return true;
}

[apoelstra]

Hmm. Ok, I continue to maintain that this is a bug in Core, but it seems like it might be by design.

[brunoerg]

Hmm. Ok, I continue to maintain that this is a bug in Core, but it seems like it might be by design.

Yes, I agree. Just was trying to understand what is the cause.

[brunoerg]

Opened bitcoin/bitcoin#31603 to fix it.