patricklaf / SNMP

Simple Network Management Protocol library for Arduino
MIT License
10 stars 1 forks source link

Incorrect representation of negative numbers #5

Closed metaathron closed 7 months ago

metaathron commented 8 months ago

Hi again, I have found another one imperfection in your code. Negative values in IntegerBER cause infinite loop and also numbers which have higest set bit on any 8th position are represented as negative (e.g. 0000 0000 1001 0110 or 1111 1111 1011 1000)

Both can be solved by this code:

void setValue(const int32_t value) {
    _value = value < 0 ? -value : value;
    // L
    _length = 0;
    char _carry;
    do {
        _carry = _value & 0x80;
        _value >>= 8;
        _length++;
    } while (_value | _carry);
    // V
    _value = value;
    _size = _length + 2;
}

It checks if highest bit of byte is 1 (_carry), then it continues adding another byte to SNMP representation of number. If the number is negative, it flips it (ternary operator) and proceeds to check length which is afaik the simplest solution.

patricklaf commented 8 months ago

Hi @metaathron , Thanks for caching this one. I will have a look at it in a few days.

patricklaf commented 8 months ago

I created a PR #6, can you test it? I have tested with theses values on a Mega 2560. Encoding and decoding is OK.

Integer value BER encoding
0 02 01 00
127 02 01 7F
128 02 02 00 80
256 02 02 01 00
-128 02 01 80
-129 02 02 FF 7F

Table 3. Example BER encodings of INTEGER values from A Layman's Guide to a Subset of ASN.1, BER, and DER

patricklaf commented 7 months ago

PR #6 merged. Issue solved. @metaathron , thanks a lot for your help.

metaathron commented 7 months ago

You're very welcome. Also thanks for your job.