* some more fixes to OID codecs

* explicit limit on ObjectIdentifier arc value size removed
* some more tests in OID encoding problem areas added

5 files changed, 213 insertions, 66 deletions

diff --git a/CHANGES b/CHANGES
index 414efc5..f55d9c6 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,7 @@ Revision 0.1.8
 --------------
 
 - ObjectIdentifier codec fixed to work properly with arc 0 and arc 2 values.
+- Explicit limit on ObjectIdentifier arc value size removed.
 - Unicode initializer support added to OctetString type and derivatives.
 - Fix to SEQUENCE and SET types to give them their private componentTypes
   collection (which is a NamedTypes object) so that they won't collide in
diff --git a/pyasn1/codec/ber/decoder.py b/pyasn1/codec/ber/decoder.py
index 1530a54..d0c9814 100644
--- a/pyasn1/codec/ber/decoder.py
+++ b/pyasn1/codec/ber/decoder.py
@@ -216,30 +216,14 @@ class ObjectIdentifierDecoder(AbstractSimpleDecoder):
         if not head:
             raise error.PyAsn1Error('Empty substrate')
 
-        # Decode the first octet
-        index = 1
-        subId = oct2int(head[0])
-        if 0 <= subId <= 39:
-            oid = 0, subId
-        elif 40 <= subId <= 79:
-            oid = 1, subId - 40
-        elif subId >= 80:
-            oid = 2,
-            index = 0
-        else:
-            raise error.PyAsn1Error('Malformed first OID octet: %s' % subId)
-
+        oid = ()
+        index = 0
         substrateLen = len(head)
         while index < substrateLen:
             subId = oct2int(head[index])
-            if index == 0:
-                subId -= 80
-            index = index + 1
-            if subId == 128:
-                # ASN.1 spec forbids leading zeros (0x80) in sub-ID OID
-                # encoding, tolerating it opens a vulnerability.
-                # See http://www.cosic.esat.kuleuven.be/publications/article-1432.pdf page 7
-                raise error.PyAsn1Error('Invalid leading 0x80 in sub-OID')
+            index += 1
+            if subId < 128:
+                oid = oid + (subId,)
             elif subId > 128:
                 # Construct subid from a number of octets
                 nextSubId = subId
@@ -249,11 +233,27 @@ class ObjectIdentifierDecoder(AbstractSimpleDecoder):
                     if index >= substrateLen:
                         raise error.SubstrateUnderrunError(
                             'Short substrate for sub-OID past %s' % (oid,)
-                            )
+                        )
                     nextSubId = oct2int(head[index])
-                    index = index + 1
-                subId = (subId << 7) + nextSubId
-            oid = oid + (subId,)
+                    index += 1
+                oid = oid + ((subId << 7) + nextSubId,)
+            elif subId == 128:
+                # ASN.1 spec forbids leading zeros (0x80) in OID
+                # encoding, tolerating it opens a vulnerability. See
+                # http://www.cosic.esat.kuleuven.be/publications/article-1432.pdf
+                # page 7
+                raise error.PyAsn1Error('Invalid octet 0x80 in OID encoding')
+       
+        # Decode two leading arcs
+        if 0 <= oid[0] <= 39:
+            oid = (0,) + oid
+        elif 40 <= oid[0] <= 79:
+            oid = (1, oid[0]-40) + oid[1:]
+        elif oid[0] >= 80:
+            oid = (2, oid[0]-80) + oid[1:]
+        else:
+            raise error.PyAsn1Error('Malformed first OID octet: %s' % head[0])
+
         return self._createComponent(asn1Spec, tagSet, oid), tail
 
 class RealDecoder(AbstractSimpleDecoder):
diff --git a/pyasn1/codec/ber/encoder.py b/pyasn1/codec/ber/encoder.py
index a52f9da..b19f240 100644
--- a/pyasn1/codec/ber/encoder.py
+++ b/pyasn1/codec/ber/encoder.py
@@ -156,48 +156,46 @@ class ObjectIdentifierEncoder(AbstractItemEncoder):
     precomputedValues = {
         (1, 3, 6, 1, 2): (43, 6, 1, 2),        
         (1, 3, 6, 1, 4): (43, 6, 1, 4)
-        }
+    }
     def encodeValue(self, encodeFun, value, defMode, maxChunkSize):    
         oid = value.asTuple()
         if oid[:5] in self.precomputedValues:
+            oid = oid[5:]
             octets = self.precomputedValues[oid[:5]]
-            index = 5
         else:
             if len(oid) < 2:
                 raise error.PyAsn1Error('Short OID %s' % (value,))
 
-            index = 2
+            octets = ()
 
             # Build the first twos
             if oid[0] == 0 and 0 <= oid[1] <= 39:
-                octets = (oid[1],)
+                oid = (oid[1],) + oid[2:]
             elif oid[0] == 1 and 0 <= oid[1] <= 39:
-                octets = (oid[1] + 40,)
+                oid = (oid[1] + 40,) + oid[2:]
             elif oid[0] == 2:
-                octets = ()
                 oid = (oid[1] + 80,) + oid[2:]
-                index = 0
             else:
                 raise error.PyAsn1Error(
-                    'Initial sub-ID overflow %s in OID %s' % (oid[:2], value)
+                    'Impossible initial arcs %s at %s' % (oid[:2], value)
                     )
 
-        # Cycle through subids
-        for subid in oid[index:]:
-            if subid > -1 and subid < 128:
+        # Cycle through subIds
+        for subId in oid:
+            if subId > -1 and subId < 128:
                 # Optimize for the common case
-                octets = octets + (subid & 0x7f,)
-            elif subid < 0:
+                octets = octets + (subId & 0x7f,)
+            elif subId < 0:
                 raise error.PyAsn1Error(
-                    'SubId overflow %s in %s' % (subid, value)
-                    )
+                    'Negative OID arc %s at %s' % (subId, value)
+                )
             else:
                 # Pack large Sub-Object IDs
-                res = (subid & 0x7f,)
-                subid = subid >> 7
-                while subid > 0:
-                    res = (0x80 | (subid & 0x7f),) + res
-                    subid = subid >> 7 
+                res = (subId & 0x7f,)
+                subId = subId >> 7
+                while subId > 0:
+                    res = (0x80 | (subId & 0x7f),) + res
+                    subId = subId >> 7 
                 # Add packed Sub-Object ID to resulted Object ID
                 octets += res
 
diff --git a/test/codec/ber/test_decoder.py b/test/codec/ber/test_decoder.py
index 294e0ce..ec58bde 100644
--- a/test/codec/ber/test_decoder.py
+++ b/test/codec/ber/test_decoder.py
@@ -179,39 +179,66 @@ class NullDecoderTestCase(unittest.TestCase):
         else:
             assert 0, 'wrong tagFormat worked out'
 
+# Useful analysis of OID encoding issues could be found here:
+# http://www.viathinksoft.de/~daniel-marschall/asn.1/oid_facts.html
 class ObjectIdentifierDecoderTestCase(unittest.TestCase):
-    def testOID(self):
+    def testOne(self):
         assert decoder.decode(
             ints2octs((6, 6, 43, 6, 0, 191, 255, 126))
-            ) == ((1,3,6,0,0xffffe), null)
+        ) == ((1,3,6,0,0xffffe), null)
 
-    def testEdges1(self):
+    def testEdge1(self):
         assert decoder.decode(
             ints2octs((6, 1, 39))
-            ) == ((0,39), null)
+        ) == ((0,39), null)
 
-    def testEdges2(self):
+    def testEdge2(self):
         assert decoder.decode(
             ints2octs((6, 1, 79))
-            ) == ((1,39), null)
+        ) == ((1,39), null)
 
-    def testEdges3(self):
+    def testEdge3(self):
         assert decoder.decode(
-            ints2octs((6, 5, 223, 255, 255, 255, 127))
-            ) == ((2,0xffffffff), null)
+            ints2octs((6, 1, 120))
+        ) == ((2,40), null)
 
+    def testEdge4(self):
+        assert decoder.decode(
+            ints2octs((6,5,0x90,0x80,0x80,0x80,0x4F))
+        ) == ((2,0xffffffff), null)
+
+    def testEdge5(self):
+        assert decoder.decode(
+            ints2octs((6,1,0x7F))
+        ) == ((2,47), null)
+
+    def testEdge6(self):
+        assert decoder.decode(
+            ints2octs((6,2,0x81,0x00))
+        ) == ((2,48), null)
+
+
+    def testEdge7(self):
+        assert decoder.decode(
+            ints2octs((6,3,0x81,0x34,0x03))
+        ) == ((2,100,3), null)
+
+    def testEdge8(self):
+        assert decoder.decode(
+            ints2octs((6,2,133,0))
+        ) == ((2,560), null)
 
-    def testEdges3(self):
+    def testEdge9(self):
         assert decoder.decode(
-            ints2octs((6, 7, 43, 6, 143, 255, 255, 255, 127))
-            ) == ((1, 3, 6, 4294967295), null)
+            ints2octs((6,4,0x88,0x84,0x87,0x02))
+        ) == ((2,16843570), null)
 
     def testNonLeading0x80(self):
         assert decoder.decode(
             ints2octs((6, 5, 85, 4, 129, 128, 0)),
-            ) == ((2, 5, 4, 16384), null)
+        ) == ((2, 5, 4, 16384), null)
 
-    def testLeading0x80(self):
+    def testLeading0x80Case1(self):
         try:
             decoder.decode(
                 ints2octs((6, 5, 85, 4, 128, 129, 0))
@@ -219,7 +246,37 @@ class ObjectIdentifierDecoderTestCase(unittest.TestCase):
         except PyAsn1Error:
             pass
         else:
-            assert 1, 'Leading 0x80 tolarated'
+            assert 0, 'Leading 0x80 tolarated'
+
+    def testLeading0x80Case2(self):
+        try:
+            decoder.decode(
+                ints2octs((6,7,1,0x80,0x80,0x80,0x80,0x80,0x7F))
+            )
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'Leading 0x80 tolarated'
+
+    def testLeading0x80Case3(self):
+        try:
+            decoder.decode(
+                ints2octs((6,2,0x80,1))
+            )
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'Leading 0x80 tolarated'
+
+    def testLeading0x80Case4(self):
+        try:
+            decoder.decode(
+                ints2octs((6,2,0x80,0x7F))
+            )
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'Leading 0x80 tolarated'
 
     def testTagFormat(self):
         try:
@@ -229,6 +286,48 @@ class ObjectIdentifierDecoderTestCase(unittest.TestCase):
         else:
             assert 0, 'wrong tagFormat worked out'
 
+    def testZeroLength(self):
+        try:
+            decoder.decode(ints2octs((6, 0, 0)))
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'zero length tolarated'
+
+    def testIndefiniteLength(self):
+        try:
+            decoder.decode(ints2octs((6, 128, 0)))
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'indefinite length tolarated'
+
+    def testReservedLength(self):
+        try:
+            decoder.decode(ints2octs((6, 255, 0)))
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'reserved length tolarated'
+
+    def testReservedLength(self):
+        try:
+            decoder.decode(ints2octs((6, 255, 0)))
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'reserved length tolarated'
+
+    def testLarge1(self):
+        assert decoder.decode(
+            ints2octs((0x06,0x11,0x83,0xC6,0xDF,0xD4,0xCC,0xB3,0xFF,0xFF,0xFE,0xF0,0xB8,0xD6,0xB8,0xCB,0xE2,0xB7,0x17))
+        ) == ((2,18446744073709551535184467440737095), null)
+
+    def testLarge2(self):
+        assert decoder.decode(
+            ints2octs((0x06,0x13,0x88,0x37,0x83,0xC6,0xDF,0xD4,0xCC,0xB3,0xFF,0xFF,0xFE,0xF0,0xB8,0xD6,0xB8,0xCB,0xE2,0xB6,0x47))
+        ) == ((2,999,18446744073709551535184467440737095), null)
+
 class RealDecoderTestCase(unittest.TestCase):
     def testChar(self):
         assert decoder.decode(
diff --git a/test/codec/ber/test_encoder.py b/test/codec/ber/test_encoder.py
index b35f326..3bd0509 100644
--- a/test/codec/ber/test_encoder.py
+++ b/test/codec/ber/test_encoder.py
@@ -148,41 +148,90 @@ class ObjectIdentifierEncoderTestCase(unittest.TestCase):
         ) == ints2octs((6,1,79))
 
     def testEdge3(self):
+        #01111111
+        assert encoder.encode(
+            univ.ObjectIdentifier((2,40))
+        ) == ints2octs((6,1,120))
+
+    def testEdge4(self):
         #10010000|10000000|10000000|10000000|01001111
         assert encoder.encode(
             univ.ObjectIdentifier((2,0xffffffff))
         ) == ints2octs((6,5,0x90,0x80,0x80,0x80,0x4F))
 
-    def testEdge4(self):
+    def testEdge5(self):
         #01111111
         assert encoder.encode(
             univ.ObjectIdentifier((2,47))
         ) == ints2octs((6,1,0x7F))
 
-    def testEdge5(self):
+    def testEdge6(self):
         #10000001|00000000
         assert encoder.encode(
             univ.ObjectIdentifier((2,48))
         ) == ints2octs((6,2,0x81,0x00))
 
-    def testEdge6(self):
+    def testEdge7(self):
         #10000001|00110100|00000003
         assert encoder.encode(
             univ.ObjectIdentifier((2,100,3))
         ) == ints2octs((6,3,0x81,0x34,0x03))
 
-    def testEdge7(self):
+    def testEdge8(self):
         #10000101|00000000
         assert encoder.encode(
             univ.ObjectIdentifier((2,560))
-        ) == ints2octs((6,2,133,00))
+        ) == ints2octs((6,2,133,0))
 
-    def testEdge8(self):
+    def testEdge9(self):
         #10001000|10000100|10000111|0000010
         assert encoder.encode(
             univ.ObjectIdentifier((2,16843570))
         ) == ints2octs((6,4,0x88,0x84,0x87,0x02))
 
+    def testImpossible1(self):
+        try:
+            encoder.encode(univ.ObjectIdentifier((3,1,2)))
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'impossible leading arc tolerated'
+
+    def testImpossible2(self):
+        try:
+            encoder.encode(univ.ObjectIdentifier((0,)))
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'single arc OID tolerated'
+
+    def testImpossible3(self):
+        try:
+            encoder.encode(univ.ObjectIdentifier((0,40)))
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'second arc overflow tolerated'
+
+    def testImpossible4(self):
+        try:
+            encoder.encode(univ.ObjectIdentifier((1,40)))
+        except PyAsn1Error:
+            pass
+        else:
+            assert 0, 'second arc overflow tolerated'
+
+    def testLarge1(self):
+        assert encoder.encode(
+            univ.ObjectIdentifier((2,18446744073709551535184467440737095))
+        ) == ints2octs((0x06,0x11,0x83,0xC6,0xDF,0xD4,0xCC,0xB3,0xFF,0xFF,0xFE,0xF0,0xB8,0xD6,0xB8,0xCB,0xE2,0xB7,0x17))
+
+    def testLarge2(self):
+        assert encoder.encode(
+            univ.ObjectIdentifier((2,999,18446744073709551535184467440737095))
+        ) == ints2octs((0x06,0x13,0x88,0x37,0x83,0xC6,0xDF,0xD4,0xCC,0xB3,0xFF,0xFF,0xFE,0xF0,0xB8,0xD6,0xB8,0xCB,0xE2,0xB6,0x47))
+            
+
 class RealEncoderTestCase(unittest.TestCase):
     def testChar(self):
         assert encoder.encode(