On 10/3/19 10:33 AM, Sebastien Marie wrote:
> On Thu, Oct 03, 2019 at 10:01:06AM +0200, Martijn van Duren wrote:
>>>> Index: snmpc.c
>>>> ===================================================================
>>>> RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
>>>> retrieving revision 1.11
>>>> diff -u -p -r1.11 snmpc.c
>>>> --- snmpc.c 18 Sep 2019 09:54:36 -0000 1.11
>>>> +++ snmpc.c 26 Sep 2019 12:32:38 -0000
>>>> @@ -666,19 +669,54 @@ snmpc_walk(int argc, char *argv[])
>>>> }
>>>>
>>>> int
>>>> +snmpc_set(int argc, char *argv[])
>>>> +{
>>>> + struct snmp_agent *agent;
>>>> + struct ber_element *pdu, *varbind;
>>>> + int errorstatus, errorindex;
>>>> + int class;
>>>> + unsigned type;
>>>> +
>>>> + if (argc < 4)
>>>> + usage();
>>>> + if ((agent = snmpc_connect(argv[0], "161")) == NULL)
>>>> + err(1, "%s", snmp_app->name);
>>>> + argc--;
>>>> + argv++;
>>>> +
>>>> + if (argc < 3 || argc % 3 != 0)
>>>> + usage();
>>>> +
>>>> + if (pledge("stdio", NULL) == -1)
>>>> + err(1, "pledge");
>>>> +
>>>> + pdu = snmp_set(agent, snmpc_varbindparse(argc, argv));
>>>> +
>>>> + (void) ber_scanf_elements(pdu, "t{Sdd{e", &class, &type, &errorstatus,
>>>> + &errorindex, &varbind);
>>>> + if (errorstatus != 0)
>>>> + snmpc_printerror((enum snmp_error) errorstatus,
>>>> + argv[errorindex - 1]);
>>>
>>> is "errorindex - 1" the right index ?
>>
>> Yes: RFC 3416 section 4.1:
>> A variable binding is identified by its index value. The first variable
>> binding in a variable-binding list is index one, the second is index
>> two, etc.
>>
>> I choose to use argv, because it's easier for the end user to see his
>> own input for where he mistyped instead of a double parsed oid to -O
>> based output.
>>
>>> $ ./obj/snmp set 192.168.1.5 sysContact.0 s "test"
>>> snmp: Can't parse oid 192.168.1.5: Not writable
>>>
>>> Note that the same pattern is used in others commands.
>>
>> Somehow the machines I test against don't return a not writable message.
>> Tested against snmpd, net-snmpd, HP Laserjet.
>
> The agent is on a TOSHIBA e-STUDIO3505AC device.
>
>> Could you show me a tcpdump -v output on this output?
>
> It returns 0 as index.
>
> for the command: ./obj/snmp set 192.168.1.5 sysContact.0 s "test"
>
> $ tcpdump -r test.pcap -n
> 10:06:12.953745 192.168.1.12.4864 > 192.168.1.5.161: SetRequest(32)
> .1.3.6.1.2.1.1.4.0="test"
> 10:06:12.954552 192.168.1.5.161 > 192.168.1.12.4864: GetResponse(32)
> notWritable[errorIndex==0] .1.3.6.1.2.1.1.4.0="test" (DF)
>
> $ tcpdump -r test.pcap -nvvv
> 10:06:12.953745 192.168.1.12.4864 > 192.168.1.5.161: [udp sum ok]
> |30|2d|02|01SNMPv2c |04|06|a3|20SetRequest(32)|02|04
> reqId=233006489|02|01|02|01|30|12 |30|10|06|08.1.3.6.1.2.1.1.4.0=|04|04"test"
> (ttl 64, id 50468, len 75, bad ip cksum 0! -> 7c1b)
> 10:06:12.954552 192.168.1.5.161 > 192.168.1.12.4864: [udp sum ok]
> |30|2d|02|01SNMPv2c |04|06|a2|20GetResponse(32)|02|04 reqId=233006489|02|01
> notWritable|02|01[errorIndex==0]|30|12
> |30|10|06|08.1.3.6.1.2.1.1.4.0=|04|04"test" (DF) (ttl 64, id 20430, len 75)
>
> And after more check, the index problem is without problem on toshiba side:
> depending the community used, the returned oid (and index) is wrong.
>
> $ snmp set -c private 192.168.1.5 sysDescr.0 s "test"
> snmp: Can't parse oid sysDescr.0: Not writable
>
> ./obj/snmp set -c public 192.168.1.5 sysDescr.0 s "test"
> snmp: Can't parse oid 192.168.1.5: Not writable
>
> (I changed the oid because with 'private' I am able to write to sysContact.0)
>
>
> But even if the agent is wrong, I think it is dangerous to bindly trust the
> other
> side of accessing a memory chunk. Before using the value, it should be checked
> against the valid range to avoid uncontrolled memory access.
>
> For me, for such ill response, snmp should exit with error.
>
> FYI, snmpset does the following:
>
> $ snmpset -v2c -c public 192.168.92.5 sysDescr.0 = "test"
> Error in packet.
> Reason: notWritable (That object does not support modification)
>
> Thanks.
>
I agree, but considering this is also part of all the other command, OK
to commit this as is and fix it for all the other in one sweep?
