search

openwisp / openwrt-openwisp-monitoring

OpenWRT monitoring agent for openwisp-monitoring
https://openwisp.io/docs/dev/openwrt-monitoring-agent/
GNU General Public License v3.0
23 stars 20 forks source link

[bug] 'pairs' on unexpected variable value #119

Open labertasche42 opened 1 year ago

labertasche42 commented 1 year ago

I have too many devices and thus even in the normal openwrt 22 cli, ubus call network.device status throws a time out. Entirely my problem, but through this time out https://github.com/openwisp/openwrt-openwisp-monitoring/blob/6709f353f1bd3a99282e9e656e29b5489cd4f1c4/openwisp-monitoring/files/sbin/netjson-monitoring.lua#L65 gets an unexpected assignment and by this https://github.com/openwisp/openwrt-openwisp-monitoring/blob/6709f353f1bd3a99282e9e656e29b5489cd4f1c4/openwisp-monitoring/files/sbin/netjson-monitoring.lua#L123 fails. This in turn results in an empty report in this in a HTTP 400 from the openwisp server.

The solution could be to either check every variable, before pairs is used on it and omit this part in the report, if there should be an error, or to abort the whole script, if ubus:call has an error.

nemesifier commented 1 year ago

@labertasche42 interesting! Does it always timeout or if you keep repeating it would work?

labertasche42 commented 1 year ago

Hey @nemesisdesign,

as every timeout is 30 seconds I just did test 6 calls, but yes, even after 6 calls it always times out. But this is a problem of the ubus command line tool and should probably a report somewhere else.

openwrt-openwisp-monitoring's Line 123 and Line 112's problem is missing input sanitization which subsequently led in my case to errors like #101 or #102 In my opinion it should be something like

if type(x) == "table" then 
for a, b in pairs(x) do
...
else
 [errorreport]

For my case, I changed L65 to local network_status = {} for now to get at least all the other informations. A filter in the ubus:call in L65 would have worked too, but I was too lazy to recode L124ff