ber.c: Fix some minor issues in ober_read_element()

[Martijn van Duren]

I found 2 minor issues in the handling of sequences/sets in
ober_read_element():
1) An empty sequence/set (which is basically always) unconditionally
   creates an (uninitialised) sub-element. Add the same length check
   used to check the next element
2) For each sub-element r is only checked for -1, but not if it
   overflows the length of the sequence itself. This is not a big risk
   since each sub-element is length-checked against the buffer
   availability and simply returns an ECANCELED, which would be no
   worse memory-wise than sending an extremely large packet.

While here, only having the NULL of the comparison on the next line
annoyed me.

OK?

martijn@

Index: ber.c
===================================================================
RCS file: /cvs/src/lib/libutil/ber.c,v
retrieving revision 1.23
diff -u -p -r1.23 ber.c
--- ber.c       21 Oct 2021 08:17:33 -0000      1.23
+++ ber.c       2 Nov 2022 06:28:33 -0000
@@ -1375,7 +1375,7 @@ ober_read_element(struct ber *ber, struc
                break;
        case BER_TYPE_SEQUENCE:
        case BER_TYPE_SET:
-               if (elm->be_sub == NULL) {
+               if (len > 0 && elm->be_sub == NULL) {
                        if ((elm->be_sub = ober_get_element(0)) == NULL)
                                return -1;
                }
@@ -1390,13 +1390,21 @@ ober_read_element(struct ber *ber, struc
                                return -1;
                        }
                        r = ober_read_element(ber, next);
-                       if (r == -1)
+                       if (r == -1) {
+                               /* sub-element overflows sequence/set */
+                               if (errno == ECANCELED)
+                                       errno = EINVAL;
                                return -1;
+                       }
+                       if (r > len) {
+                               errno = EINVAL;
+                               return -1;
+                       }
                        elements++;
                        len -= r;
                        if (len > 0 && next->be_next == NULL) {
-                               if ((next->be_next = ober_get_element(0)) ==
-                                   NULL)
+                               next->be_next = ober_get_element(0);
+                               if (next->be_next == NULL)
                                        return -1;
                        }
                        next = next->be_next;