Re: how should agents handle an 'out-of-range' index ?

On Wed, Sep 15, 2010 at 11:23 AM, Dave Shield <[email protected]>wrote:

> On 14 September 2010 16:55, Fulko Hew <[email protected]> wrote:
> > When I ran this test, the agent died and needed to be restarted.
> > The variable I was querying was:
> >
> > 1.3.6.1.4.1.2021.8.1.103.4294967295  aka 'UCD-SNMP-MIB:extIndex'.
>
> Do you get the same problem when querying
>    1.3.6.1.4.1.2021.8.1.103.4294967294
> ?
>

Yes, getnext on  4294967294 also core dumps.
However get' on 4294967294 doesn't dump.


I'm somewhat suspicious that 4294967295 is 0xFFFFFFFF,
> i.e. the maximum possible 32-bit value.  So incrementing this
> (to look for the next row) might well trigger 32-bit overflow issues.
>
>
> > a) The definition of the variable says that the valid range is:
> >     SYNTAX  Integer32 (0..65535)
>
> I doubt that the problem is related to the integer sub-range.
> But you could easily check this using an index greater than 65535
> but less than 2^32-1
>

A quick test shows that:
10,000 works
100,000 works
1,000,000 works
...
2^31 -2 works
2^31 -1 (and above fails)

I don't think there is any paranoia code in there to prevent accessing
beyond the size of the current table ;-() regardless of what the SYNTAX
definition, so

I made quick patches in agent/mibgroup/agent/extend.c to the definitions of:
idx,
num_compatability_entries,
max_compatability_entries,
line_idx

from 'int' to 'unsigned int'
and added the check I proposed in another email,
and it seemed to avoid the problem, but
it may not be the 'correct' answer.

I've attached a file with the diffs I tried.


> b) the net-snmp's snmpgetnext command line tool prevents me
>     from using a BIG index

  $man snmpcmd
>
>  INPUT OPTIONS
>       -Ir    disables checking table indexes ....
>               against the relevant MIB definitions.
>

Yes, I found that when I wanted to be able to provide a Net-SNMP (only)
based bug report.

------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev

_______________________________________________
Net-snmp-users mailing list
[email protected]
Please see the following page to unsubscribe or change other options:
https://lists.sourceforge.net/lists/listinfo/net-snmp-users

--- /root/Desktop/net-snmp-5.6.rc2/agent/mibgroup/agent/extend.c	2010-07-08 07:19:15.000000000 -0400
+++ /root/Desktop/net-snmp-5.6.rc2/extend.c.new	2010-09-15 11:51:00.000000000 -0400
@@ -35,13 +35,13 @@
 
 #ifndef USING_UCD_SNMP_EXTENSIBLE_MODULE
 typedef struct netsnmp_old_extend_s {
-    int idx;
+    unsigned int idx;
     netsnmp_extend *exec_entry;
     netsnmp_extend *efix_entry;
 } netsnmp_old_extend;
 
-int             num_compatability_entries = 0;
-int             max_compatability_entries = 50;
+unsigned int             num_compatability_entries = 0;
+unsigned int             max_compatability_entries = 50;
 netsnmp_old_extend *compatability_entries;
 
 WriteMethod fixExec2Error;
@@ -1098,7 +1098,7 @@
 {
     netsnmp_extend            *eptr;
     extend_registration_block *ereg;
-    int line_idx;
+    unsigned int line_idx;
     oid oid_buf[MAX_OID_LEN];
     int oid_len;
     int i;
@@ -1276,7 +1276,7 @@
     netsnmp_table_request_info *table_info;
     netsnmp_extend             *extension;
     char *cp;
-    int line_idx;
+    unsigned int line_idx;
     int len;
 
     for ( request=requests; request; request=request->next ) {
@@ -1353,13 +1353,15 @@
 {
     netsnmp_old_extend *exten = NULL;
     static long     long_ret;
-    int idx;
+    unsigned int idx;
 
     if (header_simple_table
         (vp, name, length, exact, var_len, write_method, num_compatability_entries))
         return (NULL);
 
     idx = name[*length-1] -1;
+	if (idx > max_compatability_entries)
+		return NULL;
     exten = &compatability_entries[ idx ];
     if (exten) {
         switch (vp->magic) {
@@ -1415,7 +1417,7 @@
              u_char * statP, oid * name, size_t name_len)
 {
     netsnmp_old_extend *exten = NULL;
-    int idx;
+    unsigned int idx;
 
     idx = name[name_len-1] -1;
     exten = &compatability_entries[ idx ];