This one has been irking me for some time. At the moment when decoding a
ber message there is no way for br_application to indicate that the
tag/type is unrecognised and if br_application is not set we default to
BER_TYPE_NULL. In basically all cases any wrong encoding should be
caught by ober_scanf_elements and friends, but I'd rather fail sooner
than later.

By scanning the tree I found BER_TYPE_DEFAULT only used by ber.c, so I
decided to rename it to BER_TYPE_UNKNOWN so that it can be used as a
failure return value from br_application. The value of -1 can safely be
used here, since get_id() allows max sizeof(unsigned int) iterations and
each iteration adds 7 bits to the value, so we never reach negative
numbers through natural means.

In a similar fashion: don't simply ignore elements in
ober_write_elements() if the encoding is not recognised.

ober_set_application() is currently not documented, so on this front
there is no update, I think this should be done as a separate endeavour.

I've put this diff through snmp, snmpd, ldapd, libutil regress without
problems, but I can't fully rule out any fallout (valid, or invalid).

thoughts? OK?

martijn@

Index: ber.c
===================================================================
RCS file: /cvs/src/lib/libutil/ber.c,v
retrieving revision 1.24
diff -u -p -r1.24 ber.c
--- ber.c       3 Nov 2022 17:58:10 -0000       1.24
+++ ber.c       23 Dec 2022 11:42:38 -0000
@@ -64,7 +64,7 @@ ober_get_element(unsigned int encoding)
                return NULL;
 
        elm->be_encoding = encoding;
-       ober_set_header(elm, BER_CLASS_UNIVERSAL, BER_TYPE_DEFAULT);
+       ober_set_header(elm, BER_CLASS_UNIVERSAL, BER_TYPE_UNKNOWN);
 
        return elm;
 }
@@ -73,7 +73,7 @@ void
 ober_set_header(struct ber_element *elm, int class, unsigned int type)
 {
        elm->be_class = class & BER_CLASS_MASK;
-       if (type == BER_TYPE_DEFAULT)
+       if (type == BER_TYPE_UNKNOWN)
                type = elm->be_encoding;
        elm->be_type = type;
 }
@@ -899,7 +899,7 @@ ober_read_elements(struct ber *ber, stru
        struct ber_element *root = elm;
 
        if (root == NULL) {
-               if ((root = ober_get_element(0)) == NULL)
+               if ((root = ober_get_element(BER_TYPE_UNKNOWN)) == NULL)
                        return NULL;
        }
 
@@ -1078,6 +1078,9 @@ ober_dump_element(struct ber *ber, struc
                if (root->be_sub && ober_dump_element(ber, root->be_sub) == -1)
                        return -1;
                break;
+       default:
+               errno = EINVAL;
+               return -1;
        }
 
        if (root->be_next == NULL)
@@ -1291,7 +1294,7 @@ ober_read_element(struct ber *ber, struc
        elm->be_offs = ber->br_offs;    /* element position within stream */
        elm->be_class = class;
 
-       if (elm->be_encoding == 0) {
+       if (elm->be_encoding == BER_TYPE_UNKNOWN) {
                /* try to figure out the encoding via class, type and cstruct */
                if (cstruct)
                        elm->be_encoding = BER_TYPE_SEQUENCE;
@@ -1304,9 +1307,12 @@ ober_read_element(struct ber *ber, struc
                         * type is defined as 4 byte OCTET STRING.
                         */
                        elm->be_encoding = (*ber->br_application)(elm);
-               } else
-                       /* last resort option */
-                       elm->be_encoding = BER_TYPE_NULL;
+               }
+
+               if (elm->be_encoding == BER_TYPE_UNKNOWN) {
+                       errno = EINVAL;
+                       return -1;
+               }
        }
 
        switch (elm->be_encoding) {
@@ -1376,7 +1382,8 @@ ober_read_element(struct ber *ber, struc
        case BER_TYPE_SEQUENCE:
        case BER_TYPE_SET:
                if (len > 0 && elm->be_sub == NULL) {
-                       if ((elm->be_sub = ober_get_element(0)) == NULL)
+                       elm->be_sub = ober_get_element(BER_TYPE_UNKNOWN);
+                       if (elm->be_sub == NULL)
                                return -1;
                }
                next = elm->be_sub;
@@ -1403,7 +1410,8 @@ ober_read_element(struct ber *ber, struc
                        elements++;
                        len -= r;
                        if (len > 0 && next->be_next == NULL) {
-                               next->be_next = ober_get_element(0);
+                               next->be_next =
+                                   ober_get_element(BER_TYPE_UNKNOWN);
                                if (next->be_next == NULL)
                                        return -1;
                        }
Index: ber.h
===================================================================
RCS file: /cvs/src/lib/libutil/ber.h,v
retrieving revision 1.5
diff -u -p -r1.5 ber.h
--- ber.h       31 Oct 2021 16:42:08 -0000      1.5
+++ ber.h       23 Dec 2022 11:42:38 -0000
@@ -58,7 +58,7 @@ struct ber {
 };
 
 /* well-known ber_element types */
-#define BER_TYPE_DEFAULT       ((unsigned int)-1)
+#define BER_TYPE_UNKNOWN       ((unsigned int)-1)
 #define BER_TYPE_EOC           0
 #define BER_TYPE_BOOLEAN       1
 #define BER_TYPE_INTEGER       2

Reply via email to