From 2d30234595552f2edc9726a8dd21ab1cc63ecf8f Mon Sep 17 00:00:00 2001 From: martijn Date: Mon, 22 Feb 2021 11:31:09 +0000 Subject: [PATCH] Make use of the new '$' feature of ober_scanf_elements to enforce stricter ASN.1 verification. OK claudio@ --- usr.sbin/snmpd/snmpe.c | 12 ++++++------ usr.sbin/snmpd/traphandler.c | 15 +++++++-------- usr.sbin/snmpd/usm.c | 4 ++-- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/usr.sbin/snmpd/snmpe.c b/usr.sbin/snmpd/snmpe.c index 9d063206854..6e279b14f6d 100644 --- a/usr.sbin/snmpd/snmpe.c +++ b/usr.sbin/snmpd/snmpe.c @@ -1,4 +1,4 @@ -/* $OpenBSD: snmpe.c,v 1.69 2021/02/05 10:30:45 martijn Exp $ */ +/* $OpenBSD: snmpe.c,v 1.70 2021/02/22 11:31:09 martijn Exp $ */ /* * Copyright (c) 2007, 2008, 2012 Reyk Floeter @@ -227,7 +227,7 @@ snmpe_parse(struct snmp_message *msg) case SNMP_V2: if (env->sc_min_seclevel != 0) goto badversion; - if (ober_scanf_elements(a, "se", &comn, &msg->sm_pdu) != 0) + if (ober_scanf_elements(a, "seS$", &comn, &msg->sm_pdu) != 0) goto parsefail; if (strlcpy(msg->sm_community, comn, sizeof(msg->sm_community)) >= sizeof(msg->sm_community)) { @@ -237,7 +237,7 @@ snmpe_parse(struct snmp_message *msg) } break; case SNMP_V3: - if (ober_scanf_elements(a, "{iisi}e", + if (ober_scanf_elements(a, "{iisi$}e", &msg->sm_msgid, &msg->sm_max_msg_size, &flagstr, &msg->sm_secmodel, &a) != 0) goto parsefail; @@ -255,7 +255,7 @@ snmpe_parse(struct snmp_message *msg) goto parsefail; } - if (ober_scanf_elements(a, "{xxe", + if (ober_scanf_elements(a, "{xxeS$}$", &msg->sm_ctxengineid, &msg->sm_ctxengineid_len, &ctxname, &len, &msg->sm_pdu) != 0) goto parsefail; @@ -377,7 +377,7 @@ snmpe_parse(struct snmp_message *msg) } /* SNMP PDU */ - if (ober_scanf_elements(a, "iiie{et", + if (ober_scanf_elements(a, "iiie{et}$", &req, &errval, &erridx, &msg->sm_pduend, &msg->sm_varbind, &class, &type) != 0) { stats->snmp_silentdrops++; @@ -436,7 +436,7 @@ snmpe_parsevarbinds(struct snmp_message *msg) for (i = 1; varbind != NULL && i < SNMPD_MAXVARBIND; varbind = varbind->be_next, i++) { - if (ober_scanf_elements(varbind, "{oe}", &o, &value) == -1) { + if (ober_scanf_elements(varbind, "{oeS$}", &o, &value) == -1) { stats->snmp_inasnparseerrs++; msg->sm_errstr = "invalid varbind"; goto varfail; diff --git a/usr.sbin/snmpd/traphandler.c b/usr.sbin/snmpd/traphandler.c index 74347b684f0..ee7ce3c57a6 100644 --- a/usr.sbin/snmpd/traphandler.c +++ b/usr.sbin/snmpd/traphandler.c @@ -1,4 +1,4 @@ -/* $OpenBSD: traphandler.c,v 1.20 2021/01/22 06:33:27 martijn Exp $ */ +/* $OpenBSD: traphandler.c,v 1.21 2021/02/22 11:31:09 martijn Exp $ */ /* * Copyright (c) 2014 Bret Stephen Lambert @@ -67,7 +67,7 @@ traphandler_parse(struct snmp_message *msg) struct privsep *ps = &snmpd_env->sc_ps; struct snmp_stats *stats = &snmpd_env->sc_stats; struct ber ber = {0}; - struct ber_element *vblist = NULL, *elm, *elm2; + struct ber_element *vblist = NULL, *elm; struct ber_oid o1, o2, snmpTrapOIDOID; struct ber_oid snmpTrapOID, sysUpTimeOID; int sysUpTime; @@ -82,7 +82,7 @@ traphandler_parse(struct snmp_message *msg) goto done; break; case SNMP_C_TRAPV2: - if (ober_scanf_elements(msg->sm_pdu, "{SSe}", &elm) == -1) { + if (ober_scanf_elements(msg->sm_pdu, "{SSe}$", &elm) == -1) { stats->snmp_inasnparseerrs++; goto done; } @@ -98,7 +98,7 @@ traphandler_parse(struct snmp_message *msg) (void)ober_string2oid("1.3.6.1.2.1.1.3.0", &sysUpTimeOID); (void)ober_string2oid("1.3.6.1.6.3.1.1.4.1.0", &snmpTrapOIDOID); - if (ober_scanf_elements(vblist, "{{od}{oo}", &o1, &sysUpTime, &o2, + if (ober_scanf_elements(vblist, "{{od$}{oo$}", &o1, &sysUpTime, &o2, &snmpTrapOID) == -1 || ober_oid_cmp(&o1, &sysUpTimeOID) != 0 || ober_oid_cmp(&o2, &snmpTrapOIDOID) != 0) { @@ -107,8 +107,7 @@ traphandler_parse(struct snmp_message *msg) } (void)ober_scanf_elements(vblist, "{Se", &elm); for (elm = elm->be_next; elm != NULL; elm = elm->be_next) { - if (ober_scanf_elements(elm, "{oe}", &o1, &elm2) == -1 || - elm2->be_next != NULL) { + if (ober_scanf_elements(elm, "{oS$}", &o1) == -1) { stats->snmp_inasnparseerrs++; goto done; } @@ -153,7 +152,7 @@ traphandler_v1translate(struct snmp_message *msg, int proxy) int generic_trap, specific_trap, time_stamp; int hasaddress = 0, hascommunity = 0, hasenterprise = 0; - if (ober_scanf_elements(msg->sm_pdu, "{oxddde", &enterprise, + if (ober_scanf_elements(msg->sm_pdu, "{oxdddeS$}$", &enterprise, &agent_addr, &agent_addrlen, &generic_trap, &specific_trap, &time_stamp, &vblist) == -1 || agent_addrlen != 4 || @@ -379,7 +378,7 @@ trapcmd_exec(struct trapcmd *cmd, struct sockaddr *sa, goto out; for (; vb != NULL; vb = vb->be_next) { - if (ober_scanf_elements(vb, "{oe}", &oid, &elm) == -1) + if (ober_scanf_elements(vb, "{oeS$}", &oid, &elm) == -1) goto out; if ((value = smi_print_element(elm)) == NULL) goto out; diff --git a/usr.sbin/snmpd/usm.c b/usr.sbin/snmpd/usm.c index 504d8e74dc9..c78859e92f1 100644 --- a/usr.sbin/snmpd/usm.c +++ b/usr.sbin/snmpd/usm.c @@ -1,4 +1,4 @@ -/* $OpenBSD: usm.c,v 1.17 2019/10/24 12:39:27 tb Exp $ */ +/* $OpenBSD: usm.c,v 1.18 2021/02/22 11:31:09 martijn Exp $ */ /* * Copyright (c) 2012 GeNUA mbH @@ -302,7 +302,7 @@ usm_decode(struct snmp_message *msg, struct ber_element *elm, const char **errp) smi_debug_elements(usm); #endif - if (ober_scanf_elements(usm, "{xiixpxx", &engineid, &enginelen, + if (ober_scanf_elements(usm, "{xiixpxx$", &engineid, &enginelen, &engine_boots, &engine_time, &user, &userlen, &offs2, &digest, &digestlen, &salt, &saltlen) != 0) { *errp = "cannot decode USM params"; -- 2.20.1