Re: snmp df, use smi_print_element
On Thu, 2020-09-03 at 09:49 +0200, Martijn van Duren wrote:
> At the moment it's not wise to run "snmp df" against untrusted
> instances, since it outputs the hrStorageDescr without checking the
> bytes being printed.
>
> Make use of the new DISPLAY-HINT functionality of smi.c to make sure
> the string is actually DisplayHint compliant.
>
> While here add the extra continuity-check in the second loop.
>
> OK?
>
> martijn@
>
There was still a minor alignment issue in the previous diff, since
printf looks at the bytes printed and not the width of the characters,
which gives a wrong result if there's a replacement character in use.
Diff below fixes this.
All other fields should be printable ascii only and thus safe to use
in this way.
Index: mib.h
===
RCS file: /cvs/src/usr.bin/snmp/mib.h,v
retrieving revision 1.7
diff -u -p -r1.7 mib.h
--- mib.h 8 Aug 2020 14:01:31 - 1.7
+++ mib.h 3 Sep 2020 08:54:12 -
@@ -961,7 +961,7 @@
{ MIBDECL(hrStorageEntry) },\
{ MIBDECL(hrStorageIndex) },\
{ MIBDECL(hrStorageType) }, \
- { MIBDECL(hrStorageDescr) },\
+ { MIBDECL(hrStorageDescr), "DisplayString" }, \
{ MIBDECL(hrStorageAllocationUnits) }, \
{ MIBDECL(hrStorageSize) }, \
{ MIBDECL(hrStorageUsed) }, \
Index: snmpc.c
===
RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
retrieving revision 1.28
diff -u -p -r1.28 snmpc.c
--- snmpc.c 3 Aug 2020 14:45:54 - 1.28
+++ snmpc.c 3 Sep 2020 08:54:12 -
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#include "smi.h"
#include "snmp.h"
@@ -58,6 +59,7 @@ int snmpc_print(struct ber_element *);
__dead void snmpc_printerror(enum snmp_error, struct ber_element *, int,
const char *);
char *snmpc_hex2bin(char *, size_t *);
+ssize_t snmpc_mbswidth(char *);
struct ber_element *snmpc_varbindparse(int, char *[]);
void usage(void);
@@ -820,8 +822,8 @@ snmpc_df(int argc, char *argv[])
{
struct snmpc_df {
uint32_t index;
- /* DisplayString is 255a DISPLAY-HINT */
- char descr[256];
+ char *descr;
+ int descrwidth;
/* Theoretical maximum for 2 32 bit values multiplied */
char size[21];
char used[21];
@@ -833,7 +835,8 @@ snmpc_df(int argc, char *argv[])
struct ber_oid sizeoid = {{ 1, 3, 6, 1, 2, 1, 25, 2, 3, 1, 5 }, 11};
struct ber_oid usedoid = {{ 1, 3, 6, 1, 2, 1, 25, 2, 3, 1, 6 }, 11};
struct ber_oid oid, *reqoid;
- struct ber_element *pdu, *varbind;
+ char oids[SNMP_MAX_OID_STRLEN];
+ struct ber_element *pdu, *varbind, *elm;
struct snmp_agent *agent;
int errorstatus, errorindex;
int class;
@@ -890,8 +893,9 @@ snmpc_df(int argc, char *argv[])
return 1;
}
for (; varbind != NULL; varbind = varbind->be_next) {
- (void) ober_scanf_elements(varbind, "{oS", );
- if (ober_oid_cmp(, ) != 2)
+ if (ober_scanf_elements(varbind, "{os", ,
+ ) == -1 ||
+ ober_oid_cmp(, ) != 2)
break;
rows++;
}
@@ -899,19 +903,29 @@ snmpc_df(int argc, char *argv[])
err(1, "malloc");
(void) ober_scanf_elements(pdu, "{SSS{e", );
for (; i < rows; varbind = varbind->be_next, i++) {
- if (ober_scanf_elements(varbind, "{os", ,
- ) == -1) {
+ if (ober_scanf_elements(varbind, "{oe", ,
+ ) == -1) {
i--;
rows--;
continue;
}
+ if (ober_oid_cmp(, ) != 2)
+ break;
df[i].index = oid.bo_id[oid.bo_n - 1];
- len = strlcpy(df[i].descr, string,
- sizeof(df[i].descr));
- if (len > (int) sizeof(df[i].descr))
- len = (int) sizeof(df[i].descr) - 1;
- if (len > descrlen)
- descrlen = len;
+ if ((df[i].descr = smi_print_element(, elm, 0,
+ smi_os_ascii, 0, utf8)) == NULL) {
+ smi_oid2string(, oids, sizeof(oids),
+ oid_lookup);
+ warn("df: can't print oid %s", oids);
+
snmp df, use smi_print_element
At the moment it's not wise to run "snmp df" against untrusted
instances, since it outputs the hrStorageDescr without checking the
bytes being printed.
Make use of the new DISPLAY-HINT functionality of smi.c to make sure
the string is actually DisplayHint compliant.
While here add the extra continuity-check in the second loop.
OK?
martijn@
Index: mib.h
===
RCS file: /cvs/src/usr.bin/snmp/mib.h,v
retrieving revision 1.7
diff -u -p -r1.7 mib.h
--- mib.h 8 Aug 2020 14:01:31 - 1.7
+++ mib.h 3 Sep 2020 07:42:12 -
@@ -961,7 +961,7 @@
{ MIBDECL(hrStorageEntry) },\
{ MIBDECL(hrStorageIndex) },\
{ MIBDECL(hrStorageType) }, \
- { MIBDECL(hrStorageDescr) },\
+ { MIBDECL(hrStorageDescr), "DisplayString" }, \
{ MIBDECL(hrStorageAllocationUnits) }, \
{ MIBDECL(hrStorageSize) }, \
{ MIBDECL(hrStorageUsed) }, \
Index: snmpc.c
===
RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
retrieving revision 1.28
diff -u -p -r1.28 snmpc.c
--- snmpc.c 3 Aug 2020 14:45:54 - 1.28
+++ snmpc.c 3 Sep 2020 07:42:12 -
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#include "smi.h"
#include "snmp.h"
@@ -58,6 +59,7 @@ int snmpc_print(struct ber_element *);
__dead void snmpc_printerror(enum snmp_error, struct ber_element *, int,
const char *);
char *snmpc_hex2bin(char *, size_t *);
+ssize_t snmpc_mbswidth(char *);
struct ber_element *snmpc_varbindparse(int, char *[]);
void usage(void);
@@ -820,8 +822,7 @@ snmpc_df(int argc, char *argv[])
{
struct snmpc_df {
uint32_t index;
- /* DisplayString is 255a DISPLAY-HINT */
- char descr[256];
+ char *descr;
/* Theoretical maximum for 2 32 bit values multiplied */
char size[21];
char used[21];
@@ -833,7 +834,8 @@ snmpc_df(int argc, char *argv[])
struct ber_oid sizeoid = {{ 1, 3, 6, 1, 2, 1, 25, 2, 3, 1, 5 }, 11};
struct ber_oid usedoid = {{ 1, 3, 6, 1, 2, 1, 25, 2, 3, 1, 6 }, 11};
struct ber_oid oid, *reqoid;
- struct ber_element *pdu, *varbind;
+ char oids[SNMP_MAX_OID_STRLEN];
+ struct ber_element *pdu, *varbind, *elm;
struct snmp_agent *agent;
int errorstatus, errorindex;
int class;
@@ -890,8 +892,9 @@ snmpc_df(int argc, char *argv[])
return 1;
}
for (; varbind != NULL; varbind = varbind->be_next) {
- (void) ober_scanf_elements(varbind, "{oS", );
- if (ober_oid_cmp(, ) != 2)
+ if (ober_scanf_elements(varbind, "{os", ,
+ ) == -1 ||
+ ober_oid_cmp(, ) != 2)
break;
rows++;
}
@@ -899,17 +902,26 @@ snmpc_df(int argc, char *argv[])
err(1, "malloc");
(void) ober_scanf_elements(pdu, "{SSS{e", );
for (; i < rows; varbind = varbind->be_next, i++) {
- if (ober_scanf_elements(varbind, "{os", ,
- ) == -1) {
+ if (ober_scanf_elements(varbind, "{oe", ,
+ ) == -1) {
i--;
rows--;
continue;
}
+ if (ober_oid_cmp(, ) != 2)
+ break;
df[i].index = oid.bo_id[oid.bo_n - 1];
- len = strlcpy(df[i].descr, string,
- sizeof(df[i].descr));
- if (len > (int) sizeof(df[i].descr))
- len = (int) sizeof(df[i].descr) - 1;
+ if ((df[i].descr = smi_print_element(, elm, 0,
+ smi_os_ascii, 0, utf8)) == NULL) {
+ smi_oid2string(, oids, sizeof(oids),
+ oid_lookup);
+ warn("df: can't print oid %s", oids);
+ i--;
+ rows--;
+ continue;
+ }
+ if ((len = (int) snmpc_mbswidth(df[i].descr)) == -1)
+ err(1, "df: invalid hrStorageDescr");
if (len > descrlen)
descrlen = len;
}
@@ -1325,6 +1337,24 @@ fail:
errno = EINVAL;
free(decstr);
return NULL;
+}
+
+ssize_t
+snmpc_mbswidth(char *str)
+{
+ wchar_t
