Tvheadend

I have verified that it seems to occur in all API calls when entering an argument that does not exist in tvheadend code.

==19829== Thread 101 tvh:tcp-start:
==19829== Invalid read of size 1
==19829==    at 0x4C32CF2: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19829==    by 0x29D604: htsmsg_add_str (htsmsg.c:373)
==19829==    by 0x30998F: webui_api_handler (webui_api.c:39)
==19829==    by 0x273604: http_exec (http.c:1241)
==19829==    by 0x2748A9: http_cmd_get (http.c:1316)
==19829==    by 0x274AE4: http_process_request (http.c:1398)
==19829==    by 0x273C88: process_request (http.c:1533)
==19829==    by 0x274E6A: http_serve_requests (http.c:2018)
==19829==    by 0x275188: http_serve (http.c:2069)
==19829==    by 0x26B48F: tcp_server_start (tcp.c:724)
==19829==    by 0x266457: thread_wrapper (tvh_thread.c:91)
==19829==    by 0x66506DA: start_thread (pthread_create.c:463)
==19829==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Thread 102 "tvh:tcp-start" received signal SIGSEGV, Segmentation fault.
__strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:62
62      in ../sysdeps/x86_64/multiarch/strlen-avx2.S
(gdb) bt
#0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:62
#1  0x00005555556e9605 in htsmsg_add_str (msg=msg@entry=0x7fff5c00ddb0, name=0x7fff5c001010 "all", str=0x0) at src/htsmsg.c:373
#2  0x0000555555755990 in webui_api_handler (hc=0x7fff765f19b0, remain=0x7fff5c002279 "passwd/entry/grid", opaque=<optimized out>) at src/webui/webui_api.c:39
#3  0x00005555556bf605 in http_exec (hc=hc@entry=0x7fff765f19b0, hp=hp@entry=0x7fff765f1550, remain=0x7fff5c002279 "passwd/entry/grid") at src/http.c:1241
#4  0x00005555556c08aa in http_cmd_get (hc=hc@entry=0x7fff765f19b0) at src/http.c:1316
#5  0x00005555556c0ae5 in http_process_request (hc=0x7fff765f19b0, spill=<optimized out>) at src/http.c:1398
#6  0x00005555556bfc89 in process_request (hc=hc@entry=0x7fff765f19b0, spill=spill@entry=0x7fff765f1930) at src/http.c:1533
#7  0x00005555556c0e6b in http_serve_requests (hc=hc@entry=0x7fff765f19b0) at src/http.c:2018
#8  0x00005555556c1189 in http_serve (fd=99, opaque=0x7fff6c001170, peer=0x7fff6c001188, self=0x7fff6c001208) at src/http.c:2069
#9  0x00005555556b7490 in tcp_server_start (aux=0x7fff6c001140) at src/tcp.c:724
#10 0x00005555556b2458 in thread_wrapper (p=0x7fff6c0012d0) at src/tvh_thread.c:91
#11 0x00007ffff63b06db in start_thread (arg=0x7fff765f2700) at pthread_create.c:463
#12 0x00007ffff5b3388f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

NOTE: Using '?all=1' tvheadend does not crash.

It seems that the problem occurs when the option has no assigned value.

A patch could be to change the lines in webui_api.c around line 39 to

TAILQ_FOREACH(ha, &hc->hc_req_args, link) {
    if(ha->key && ha->val)
        htsmsg_add_str(args, ha->key, ha->val);
}

Entirely untested but it should work.

Flole Systems wrote:

A patch could be to change the lines in webui_api.c around line 39 to

[...]

Entirely untested but it should work.

With your patch it seems to be solved. What I do not know is if It could break some existing API call (I guess not).

diff --git a/src/webui/webui_api.c b/src/webui/webui_api.c
index 7bae1ea21..1e168695e 100644
--- a/src/webui/webui_api.c
+++ b/src/webui/webui_api.c
@@ -36,7 +36,8 @@ webui_api_handler
   /* Build arguments */
   args = htsmsg_create_map();
   TAILQ_FOREACH(ha, &hc->hc_req_args, link) {
-    htsmsg_add_str(args, ha->key, ha->val);
+    if(ha->key && ha->val)
+      htsmsg_add_str(args, ha->key, ha->val);
   }

   /* Call */

Most likely not as it only checks if one of the values is null and then ignores that parameter, if it wouldn't ignore it there's a crash.

@Jaroslav: What do you think? My proposed patch or do you think it's better to do a

htsmsg_add_str(args, ha->key ? ha->key : "", ha->val ? ha->val : "");

That would allow us to process parameters without a value like grid?tomorrow or grid?today

That check could even be moved in htsmsg_add_str.

Let's wait for his answer. An attacker could use this to block Tvheadend (if he has webui permissions)... :(