search

sr99622 / libonvif

Onvif library with GUI implementation and built in YOLOX
GNU Lesser General Public License v2.1
176 stars 45 forks source link

`onvif-util -a`: segmentation fault with non-camera devices present #106

Closed barsnick closed 3 weeks ago

barsnick commented 3 weeks ago

Hi, (recent newcomer to libonvif), I have hooked up a camera to my network, and found that onvif-util will find it when scanning, but segfault when trying to connect: Scan:

$ build-debug/onvif-util/onvif-util -a                                   
Found 2 cameras on interface 192.168.0.33
found invalid xaddrs in device repsonse
192.168.0.46 TP-LINK(Tapo C210)

Access:

$ build-debug/onvif-util/onvif-util 192.168.0.46
Segmentation fault (core dumped)

This is the backtrace from gdb:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7fb5bfd in extractHost (xaddrs=0x4276b0 "", host=0x7fffffffdda0 " k\277\367\377\177") at /home/barsnick/Development/libonvif-complete/libonvif/src/onvif.c:3034
3034                tmp[j] = xaddrs[j+start];
(gdb) bt
#0  0x00007ffff7fb5bfd in extractHost (xaddrs=0x4276b0 "", host=0x7fffffffdda0 " k\277\367\377\177") at /home/barsnick/Development/libonvif-complete/libonvif/src/onvif.c:3034
#1  0x00000000004049be in main (argc=1, argv=0x7fffffffdf88) at /home/barsnick/Development/libonvif-complete/onvif-util/src/onvif-util.cpp:276

This points to an issue similar to what is described in https://github.com/sr99622/libonvif/issues/71 and its comments: extractHost() doesn't handle empty XAddrs well. Notably this code is bad C style: https://github.com/sr99622/libonvif/blob/59effcccd30e716e4506468a5997e95f5da217af/libonvif/src/onvif.c#L3028-L3029 because it doesn't handle a returned NULL/nullptr when strstr() finds no matches.

Disregarding that: This is caused on a higher level by failing to skip devices which don't have proper XAddrs when accessing a device directly. The scan does ignore those. I will post a PR to circumvent this.

BTW, I'm not happy with the "scan" when I'm addressing a particular device anyway, but I guess that's because the UDP broadcast is required to get basic info. libonvif could ignore anything which is not from the addressed device, but that's for a later issue report or pull request. 😉

sr99622 commented 3 weeks ago

Thank you so much for your insights. The merge requests have excellent fixes in them and are very much appreciated. I really like the camera name in the prompt that is a good enhancement. You are correct about the strstr nullptr issue, this will go on the list of things to fix. The scan on addressing a device is another thing to go on the list. This has been addressed in onvif-gui, but needs to be ported back into onvif-util. The onvif-data.h file has the code in the manual_fill() member function.

I have a new release for onvif-gui that is in final test coming in the next few days that has a lot of new features that I want to get out very soon. It addresses some time sync issues that have been raised recently and has some stability fixes and expands the abilities of the proxy server. After that I can take another look at these open issues and get them fixed.

Best Regards,

Stephen