Skip to content

Promote warnings to Error in SNMP extension #6124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 15, 2020
Merged

Promote warnings to Error in SNMP extension #6124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 52 additions & 47 deletions ext/snmp/snmp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,6 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st,
*
* OID parser (and type, value for SNMP_SET command)
*/

static int php_snmp_parse_oid(
zval *object, int st, struct objid_query *objid_query, zend_string *oid_str, HashTable *oid_ht,
zend_string *type_str, HashTable *type_ht, zend_string *value_str, HashTable *value_ht
Expand All @@ -674,25 +673,33 @@ static int php_snmp_parse_oid(
objid_query->vars = (snmpobjarg *)emalloc(sizeof(snmpobjarg));
objid_query->vars[objid_query->count].oid = ZSTR_VAL(oid_str);
if (st & SNMP_CMD_SET) {
if (type_str && value_str) {
if (ZSTR_LEN(type_str) != 1) {
php_error_docref(NULL, E_WARNING, "Bogus type '%s', should be single char, got %zu", ZSTR_VAL(type_str), ZSTR_LEN(type_str));
efree(objid_query->vars);
return FALSE;
}
pptr = ZSTR_VAL(type_str);
objid_query->vars[objid_query->count].type = *pptr;
objid_query->vars[objid_query->count].value = ZSTR_VAL(value_str);
} else {
php_error_docref(NULL, E_WARNING, "Single objid and multiple type or values are not supported");
if (type_ht) {
zend_type_error("Type must be of type string when object ID is a string");
efree(objid_query->vars);
return FALSE;
}
if (value_ht) {
zend_type_error("Value must be of type string when object ID is a string");
efree(objid_query->vars);
return FALSE;
}

/* Both type and value must be valid strings */
ZEND_ASSERT(type_str && value_str);

if (ZSTR_LEN(type_str) != 1) {
zend_value_error("Type must be a single character");
efree(objid_query->vars);
return FALSE;
}
pptr = ZSTR_VAL(type_str);
objid_query->vars[objid_query->count].type = *pptr;
objid_query->vars[objid_query->count].value = ZSTR_VAL(value_str);
}
objid_query->count++;
} else if (oid_ht) { /* we got objid array */
if (zend_hash_num_elements(oid_ht) == 0) {
php_error_docref(NULL, E_WARNING, "Got empty OID array");
zend_value_error("Array of object IDs cannot be empty");
return FALSE;
}
objid_query->vars = (snmpobjarg *)safe_emalloc(sizeof(snmpobjarg), zend_hash_num_elements(oid_ht), 0);
Expand All @@ -715,7 +722,7 @@ static int php_snmp_parse_oid(
if (idx_type < type_ht->nNumUsed) {
convert_to_string_ex(tmp_type);
if (Z_STRLEN_P(tmp_type) != 1) {
php_error_docref(NULL, E_WARNING, "'%s': bogus type '%s', should be single char, got %zu", Z_STRVAL_P(tmp_oid), Z_STRVAL_P(tmp_type), Z_STRLEN_P(tmp_type));
zend_value_error("Type must be a single character");
efree(objid_query->vars);
return FALSE;
}
Expand Down Expand Up @@ -916,6 +923,7 @@ static int netsnmp_session_set_sec_level(struct snmp_session *s, char *level)
} else if (!strcasecmp(level, "authPriv") || !strcasecmp(level, "ap")) {
s->securityLevel = SNMP_SEC_LEVEL_AUTHPRIV;
} else {
zend_value_error("Security level must be one of \"noAuthNoPriv\", \"authNoPriv\", or \"authPriv\"");
return (-1);
}
return (0);
Expand All @@ -933,7 +941,7 @@ static int netsnmp_session_set_auth_protocol(struct snmp_session *s, char *prot)
s->securityAuthProto = usmHMACSHA1AuthProtocol;
s->securityAuthProtoLen = USM_AUTH_PROTO_SHA_LEN;
} else {
php_error_docref(NULL, E_WARNING, "Unknown authentication protocol '%s'", prot);
zend_value_error("Authentication protocol must be either MD5 or SHA");
return (-1);
}
return (0);
Expand All @@ -953,7 +961,11 @@ static int netsnmp_session_set_sec_protocol(struct snmp_session *s, char *prot)
s->securityPrivProtoLen = USM_PRIV_PROTO_AES_LEN;
#endif
} else {
php_error_docref(NULL, E_WARNING, "Unknown security protocol '%s'", prot);
#ifdef HAVE_AES
zend_value_error("Security protocol must be one of DES, AES128, or AES");
#else
zend_value_error("Security protocol must be DES");
#endif
return (-1);
}
return (0);
Expand Down Expand Up @@ -987,7 +999,7 @@ static int netsnmp_session_gen_sec_key(struct snmp_session *s, char *pass)
(u_char *)pass, strlen(pass),
s->securityPrivKey, &(s->securityPrivKeyLen)))) {
php_error_docref(NULL, E_WARNING, "Error generating a key for privacy pass phrase '%s': %s", pass, snmp_api_errstring(snmp_errno));
return (-2);
return (-1);
}
return (0);
}
Expand All @@ -1001,6 +1013,7 @@ static int netsnmp_session_set_contextEngineID(struct snmp_session *s, char * co
u_char *ebuf = (u_char *) emalloc(ebuf_len);

if (!snmp_hex_to_binary(&ebuf, &ebuf_len, &eout_len, 1, contextEngineID)) {
// TODO Promote to Error?
php_error_docref(NULL, E_WARNING, "Bad engine ID value '%s'", contextEngineID);
efree(ebuf);
return (-1);
Expand All @@ -1023,15 +1036,15 @@ static int netsnmp_session_set_security(struct snmp_session *session, char *sec_

/* Setting the security level. */
if (netsnmp_session_set_sec_level(session, sec_level)) {
php_error_docref(NULL, E_WARNING, "Invalid security level '%s'", sec_level);
/* ValueError already generated, just bail out */
return (-1);
}

if (session->securityLevel == SNMP_SEC_LEVEL_AUTHNOPRIV || session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) {

/* Setting the authentication protocol. */
if (netsnmp_session_set_auth_protocol(session, auth_protocol)) {
/* Warning message sent already, just bail out */
/* ValueError already generated, just bail out */
return (-1);
}

Expand All @@ -1044,7 +1057,7 @@ static int netsnmp_session_set_security(struct snmp_session *session, char *sec_
if (session->securityLevel == SNMP_SEC_LEVEL_AUTHPRIV) {
/* Setting the security protocol. */
if (netsnmp_session_set_sec_protocol(session, priv_protocol)) {
/* Warning message sent already, just bail out */
/* ValueError already generated, just bail out */
return (-1);
}

Expand Down Expand Up @@ -1220,9 +1233,9 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
snmp_object = Z_SNMP_P(object);
session = snmp_object->session;
if (!session) {
php_error_docref(NULL, E_WARNING, "Invalid or uninitialized SNMP object");
zend_throw_error(NULL, "Invalid or uninitialized SNMP object");
efree(objid_query.vars);
RETURN_FALSE;
RETURN_THROWS();
}

if (snmp_object->max_oids > 0) {
Expand Down Expand Up @@ -1351,11 +1364,9 @@ PHP_FUNCTION(snmp_set_oid_output_format)
case NETSNMP_OID_OUTPUT_NONE:
netsnmp_ds_set_int(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_OID_OUTPUT_FORMAT, a1);
RETURN_TRUE;
break;
default:
php_error_docref(NULL, E_WARNING, "Unknown SNMP output print format '%d'", (int) a1);
RETURN_FALSE;
break;
zend_argument_value_error(1, "must be an SNMP_OID_OUTPUT_* constant");
RETURN_THROWS();
}
}
/* }}} */
Expand Down Expand Up @@ -1443,8 +1454,8 @@ PHP_FUNCTION(snmp_set_valueretrieval)
SNMP_G(valueretrieval) = method;
RETURN_TRUE;
} else {
php_error_docref(NULL, E_WARNING, "Unknown SNMP value retrieval method '" ZEND_LONG_FMT "'", method);
RETURN_FALSE;
zend_argument_value_error(1, "must be a bitmask of SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, and SNMP_VALUE_OBJECT");
RETURN_THROWS();
}
}
/* }}} */
Expand Down Expand Up @@ -1827,48 +1838,46 @@ PHP_SNMP_LONG_PROPERTY_READER_FUNCTION(exceptions_enabled)
/* {{{ */
static int php_snmp_write_info(php_snmp_object *snmp_object, zval *newval)
{
php_error_docref(NULL, E_WARNING, "info property is read-only");
zend_throw_error(NULL, "SNMP::$info property is read-only");
return FAILURE;
}
/* }}} */

/* {{{ */
static int php_snmp_write_max_oids(php_snmp_object *snmp_object, zval *newval)
{
int ret = SUCCESS;
zend_long lval;

if (Z_TYPE_P(newval) == IS_NULL) {
snmp_object->max_oids = 0;
return ret;
return SUCCESS;
}

lval = zval_get_long(newval);

if (lval > 0) {
snmp_object->max_oids = lval;
} else {
php_error_docref(NULL, E_WARNING, "max_oids should be positive integer or NULL, got " ZEND_LONG_FMT, lval);
if (lval <= 0) {
zend_value_error("max_oids must be greater than 0 or null");
return FAILURE;
}
snmp_object->max_oids = lval;

return ret;
return SUCCESS;
}
/* }}} */

/* {{{ */
static int php_snmp_write_valueretrieval(php_snmp_object *snmp_object, zval *newval)
{
int ret = SUCCESS;
zend_long lval = zval_get_long(newval);

if (lval >= 0 && lval <= (SNMP_VALUE_LIBRARY|SNMP_VALUE_PLAIN|SNMP_VALUE_OBJECT)) {
snmp_object->valueretrieval = lval;
} else {
php_error_docref(NULL, E_WARNING, "Unknown SNMP value retrieval method '" ZEND_LONG_FMT "'", lval);
ret = FAILURE;
zend_value_error("SNMP retrieval method must be a bitmask of SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, and SNMP_VALUE_OBJECT");
return FAILURE;
}

return ret;
return SUCCESS;
}
/* }}} */

Expand All @@ -1892,7 +1901,6 @@ PHP_SNMP_BOOL_PROPERTY_WRITER_FUNCTION(oid_increasing_check)
/* {{{ */
static int php_snmp_write_oid_output_format(php_snmp_object *snmp_object, zval *newval)
{
int ret = SUCCESS;
zend_long lval = zval_get_long(newval);

switch(lval) {
Expand All @@ -1903,14 +1911,11 @@ static int php_snmp_write_oid_output_format(php_snmp_object *snmp_object, zval *
case NETSNMP_OID_OUTPUT_UCD:
case NETSNMP_OID_OUTPUT_NONE:
snmp_object->oid_output_format = lval;
break;
return SUCCESS;
default:
php_error_docref(NULL, E_WARNING, "Unknown SNMP output print format '" ZEND_LONG_FMT "'", lval);
ret = FAILURE;
break;
zend_value_error("SNMP output print format must be an SNMP_OID_OUTPUT_* constant");
return FAILURE;
}

return ret;
}
/* }}} */

Expand Down
44 changes: 27 additions & 17 deletions ext/snmp/tests/snmp-object-error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,35 @@ var_dump($session->close());

echo "Open normal session\n";
$session = new SNMP(SNMP::VERSION_3, $hostname, $user_noauth, $timeout, $retries);
$session->valueretrieval = 67;
var_dump($session->valueretrieval);
try {
$session->valueretrieval = 67;
var_dump($session->valueretrieval);
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
echo "Closing session\n";
var_dump($session->close());
var_dump($session->get('.1.3.6.1.2.1.1.1.0'));
var_dump($session->close());

try {
var_dump($session->get('.1.3.6.1.2.1.1.1.0'));
var_dump($session->close());
} catch (\Error $e) {
echo $e->getMessage() . \PHP_EOL;
}

$session = new SNMP(SNMP::VERSION_2c, $hostname, $community, $timeout, $retries);

var_dump($session->max_oids);
$session->max_oids = "ttt";
$session->max_oids = 0;
try {
$session->max_oids = "ttt";
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
try {
$session->max_oids = 0;
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
var_dump($session->max_oids);
?>
--EXPECTF--
Expand All @@ -81,18 +98,11 @@ int(32)
string(46) "Invalid object identifier: .1.3.6.1.2.1.1.1..0"
bool(true)
Open normal session

Warning: main(): Unknown SNMP value retrieval method '67' in %s on line %d
int(%d)
SNMP retrieval method must be a bitmask of SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, and SNMP_VALUE_OBJECT
Closing session
bool(true)

Warning: SNMP::get(): Invalid or uninitialized SNMP object in %s on line %d
bool(false)
bool(true)
Invalid or uninitialized SNMP object
NULL

Warning: main(): max_oids should be positive integer or NULL, got 0 in %s on line %d

Warning: main(): max_oids should be positive integer or NULL, got 0 in %s on line %d
max_oids must be greater than 0 or null
max_oids must be greater than 0 or null
NULL
44 changes: 21 additions & 23 deletions ext/snmp/tests/snmp-object-properties.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,24 @@ $param = 'there is no such parameter';
var_dump($session->$param);
var_dump(property_exists($session, $param));

$session->valueretrieval = 67;
var_dump($session->valueretrieval);
$session->oid_output_format = 78;
var_dump($session->oid_output_format);

$session->info = array("blah" => 2);
var_dump($session->info);
try {
$session->valueretrieval = 67;
var_dump($session->valueretrieval);
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
try {
$session->oid_output_format = 78;
var_dump($session->oid_output_format);
} catch (\ValueError $e) {
echo $e->getMessage() . \PHP_EOL;
}
try {
$session->info = array("blah" => 2);
var_dump($session->info);
} catch (\Error $e) {
echo $e->getMessage() . \PHP_EOL;
}

$session->max_oids = NULL;
var_dump($session->max_oids);
Expand Down Expand Up @@ -179,20 +190,7 @@ Error handling
Warning: Undefined property: SNMP::$there is no such parameter in %s on line %d
NULL
bool(false)

Warning: main(): Unknown SNMP value retrieval method '67' in %s on line %d
int(1)

Warning: main(): Unknown SNMP output print format '78' in %s on line %d
int(3)

Warning: main(): info property is read-only in %s on line %d
array(3) {
["hostname"]=>
string(%d) "%s"
["timeout"]=>
int(%i)
["retries"]=>
int(%d)
}
SNMP retrieval method must be a bitmask of SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, and SNMP_VALUE_OBJECT
SNMP output print format must be an SNMP_OID_OUTPUT_* constant
SNMP::$info property is read-only
NULL
Loading