Types encoding randomly fails if output buffer is not initialized to 0

[bonizzifbk]

When using the _Encode functions, it is possible that they randomly fail if the output buffer is not previously initialized to 0.

Depending on the specific circumstances, the failure can manifest in two different ways:

1. both encoding and decoding are reported as successful (return code true), but the decoded data is wrong (see TEST1 and TEST2 in the attached project)
2. the encoding is successful, but the decoding fails (return code false) (see TEST3)

When initializing the output buffer to 0, everything works fine, but in case of large data types this adds some overhead that is not negligible, especially on low-end embedded targets.

archive.zip

[bonizzifbk]

The problem seems to originate from two wrong behaviors in file _asn1crt/asn1crtencoding.c.

The first one concerns the function _BitStreamAppendNBitZero.

void BitStream_AppendNBitZero(BitStream* pBitStrm, int nbits)
{
    int totalBits = pBitStrm->currentBit + nbits;
    int totalBytes = totalBits / 8;
    pBitStrm->currentBit = totalBits % 8;
    //pBitStrm->currentByte += totalBits / 8;
    if (pBitStrm->currentByte + totalBytes <= pBitStrm->count) {
        pBitStrm->currentByte += totalBytes;
        bitstream_push_data_if_required(pBitStrm);
    } else {
        int extraBytes = pBitStrm->currentByte + totalBytes - pBitStrm->count;
        pBitStrm->currentByte = pBitStrm->count;
        bitstream_push_data_if_required(pBitStrm);
        pBitStrm->currentByte = extraBytes;
    }
}

I think this function, similarly as BitStream_AppendNBitOne, should append as many 0 bits in the pBitStrm->buf as specified by nbits. Although the pBitStrm indexes are updated, there is not an explicit assignment of the buffer to 0.

The second wrong behavior is in _BitStreamAppendPartialByte

void BitStream_AppendPartialByte(BitStream* pBitStrm, byte v, byte nbits, flag negate)
{
    int cb = pBitStrm->currentBit;
    int totalBits = cb + nbits;
    int ncb = 8 - cb;
    int totalBitsForNextByte;
    if (negate)
        v = masksb[nbits] & ((byte)~v);
    byte mask1 = (byte)~masksb[ncb];

    if (totalBits <= 8) {
        //static byte masksb[] = { 0x0, 0x1, 0x3, 0x7, 0xF, 0x1F, 0x3F, 0x7F, 0xFF };
        byte mask2 = masksb[8 - totalBits];
        byte mask = mask1 | mask2;
        //e.g. current bit = 3 --> mask =  1110 0000
        //nbits = 3 --> totalBits = 6
        //                         mask=   1110 0000
        //                         and     0000 0011 <- masks[totalBits - 1]
        //                                -----------
        //                  final mask     1110 0011
        pBitStrm->buf[pBitStrm->currentByte] &= mask;
        pBitStrm->buf[pBitStrm->currentByte] |= (byte)(v << (8 - totalBits));
        pBitStrm->currentBit += nbits;
        if (pBitStrm->currentBit == 8) {
            pBitStrm->currentBit = 0;
            pBitStrm->currentByte++;
            bitstream_push_data_if_required(pBitStrm);
        }
    }
    else {
        totalBitsForNextByte = totalBits - 8;
        pBitStrm->buf[pBitStrm->currentByte] &= mask1;
        pBitStrm->buf[pBitStrm->currentByte++] |= (byte)(v >> totalBitsForNextByte);
        bitstream_push_data_if_required(pBitStrm);
        byte mask = (byte)~masksb[8 - totalBitsForNextByte];
        pBitStrm->buf[pBitStrm->currentByte] &= mask;
        pBitStrm->buf[pBitStrm->currentByte] |= (byte)(v << (8 - totalBitsForNextByte));
        pBitStrm->currentBit = totalBitsForNextByte;
    }
    assert(pBitStrm->currentByte * 8 + pBitStrm->currentBit <= pBitStrm->count * 8);

}

In this case the bug is in the else branch when the mask for the next byte assignment is retrieved.

// masksb[] = { 0x0, 0x1, 0x3, 0x7, 0xF, 0x1F, 0x3F, 0x7F, 0xFF };
byte mask = (byte)~masksb[8 - totalBitsForNextByte];
pBitStrm->buf[pBitStrm->currentByte] &= mask;

When the mask is retrieved, it is negated, but this is wrong because with the subsequent &= operation we want to zero with the initial bits of the byte. The mask negation causes to zero the ending bits.

[usr3-1415]

For the moment, you must always call the BitStream_Init() function before calling the encode function. This function calls memset() and put zeros in the entire buffer. In near future, we will refactor these functions and this issue will be addressed.