Fix some BER decoding (#60)

* Fix some invalid decoded lengths from various

* length values >= 256 need 3 bytes to encode their length

a length of exactly 256 should be encoded as:
0x82 0x01 0x00

without this change, I was seeing the encoded length of a get response (that happened to be exactly 256 bytes long) encoded as:
0x81 0x00

that was causing snmpbulkwalk to fail, since it was reading the encoded length as 0.

* use sizeof

* fix a compiler warning treated as an error: "operation on ‘tempVal’ may be undefined"

* As documented in the comments of PR 59 ( #59 ), one of the unit test cases has an issue, resulting in false negatives.  This then prevents the automated workflow from succeeding.

***Temporarily*** disable the one unit test: "Should fail to parse a corrupt buffer"
This test needs to be modified to ensure that it always results in the intended test outcome, perhaps by not relying on random data.

---------

Co-authored-by: Aidan Cyr <aidan@aidancyr.dev>

Author: N1IOX

src/BERDecode.cpp

@@ -89,7 +89,7 @@ int IntegerType::fromBuffer(const uint8_t *buf, size_t max_len){
         break;
         case 3:
             if(tempVal & 0x00800000){
-                tempVal = tempVal |= 0xFF000000;
+                tempVal |= 0xFF000000;
             }
             _value = (int32_t)tempVal;
         break;
@@ -133,7 +133,7 @@ int OIDType::fromBuffer(const uint8_t *buf, size_t max_len){
     this->data.assign(dataPtr, dataPtr + _length);
     this->valid = true;
 
-    return _length + 2;
+    return _length + j;
 }
 
 static inline void long_to_buf(char* buf, long l, short r = 0){
@@ -208,7 +208,7 @@ int Counter64::fromBuffer(const uint8_t *buf, size_t max_len){
         _value = _value | *ptr++;
         tempLength--;
     }
-    return _length + 2;
+    return _length + i;
 }
 
 std::shared_ptr<BER_CONTAINER> ComplexType::createObjectForType(ASN_TYPE valueType){
@@ -290,5 +290,5 @@ int ComplexType::fromBuffer(const uint8_t *buf, size_t max_len){
         ptr += used_length; 
         i += used_length;
     }
-    return _length + 2;
+    return _length + j;
 }

src/BEREncode.cpp

@@ -21,7 +21,7 @@ static size_t encode_ber_length_integer(uint8_t* buf, size_t integer, int){
     if(integer < 128){
         *buf = integer & 0xFF;
     } else {
-        if(integer > 256){
+        if(integer >= 256){
             *buf++ = (2 | 0x80) & 0xFF;
             *buf++ = integer/256;
             bytes_used += 2;
@@ -37,7 +37,7 @@ static size_t encode_ber_length_integer(uint8_t* buf, size_t integer, int){
 static size_t encode_ber_length_integer_count(size_t integer){
     int bytes_used = 1;
     if(integer >= 128){
-        if(integer > 256){
+        if(integer >= 256){
             bytes_used += 2;
         } else {
             bytes_used++;

tests/tests.cpp

@@ -59,20 +59,20 @@ TEST_CASE( "Test handle failures when Encoding/Decoding", "[snmp]"){
         REQUIRE( readPack->parseFrom(buffer, 133) == SNMP_ERROR_OK );
     }
 
-    SECTION( "Should fail to parse a corrupt buffer "){
+/*    SECTION( "Should fail to parse a corrupt buffer "){
         SNMPPacket* readPacket = new SNMPPacket();
         for(int i = 25; i < 133; i+= 10){
             char old[10] = {0};
             memcpy(old, &buffer[i], 10);
             long randomLong = random();
-            memcpy(&buffer[i], &randomLong, 10);
+            memcpy(&buffer[i], &randomLong, sizeof(randomLong));
             // This may SOMETIMES fail if the random gets lucky and makes something valid
             REQUIRE( readPacket->parseFrom(buffer, 200) != SNMP_ERROR_OK );
 
             memcpy(&buffer[i], old, 10);
             REQUIRE( readPacket->parseFrom(buffer, 200) == SNMP_ERROR_OK );
         }
-    }
+    } */
 }
 
 TEST_CASE( "Test Encoding/Decoding packet", "[snmp]" ) {