Project

General

Profile

Bug #5568

API: Wrong URL makes CRASH

Added by Pablo R. over 5 years ago. Updated over 5 years ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
Category:
API
Target version:
-
Start date:
2019-03-09
Due date:
% Done:

100%

Estimated time:
Found in version:
4.3-1781~g811fd889e
Affected Versions:

Description

Using this wrong URL makes tvheadend crash instantly:

http://tvh:9981/api/mpegts/service/grid?all

CRASH:

Mar  9 12:31:52 virtual tvheadend[13221]: CRASH: Signal: 11 in PRG: /usr/bin/tvheadend (4.3-1781~g811fd889e) [2d55523585a864534ed15d3e7cbfb7b780812a8c] CWD: /
Mar  9 12:31:52 virtual tvheadend[13221]: CRASH: Fault address (nil) (Address not mapped)
Mar  9 12:31:52 virtual tvheadend[13221]: CRASH: Loaded libraries: linux-vdso.so.1 /usr/lib/x86_64-linux-gnu/libdvbcsa.so.1 /usr/lib/x86_64-linux-gnu/libssl.so.1.1 /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1 /lib/x86_64-linux-gnu/libz.so$
Mar  9 12:31:52 virtual tvheadend[13221]: CRASH: Register dump [23]: 00000000000000000000000000000000000000000000000200005640c2907bcb00007f92d4003b2000007f92d400451900007f92e77fa73000007f92d4001e80000000000000000000007f92d4003b200000000$
Mar  9 12:31:52 virtual tvheadend[13221]: CRASH: STACKTRACE
Mar  9 12:31:52 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/trap.c:176 0x5640c18f049d 0x5640c16db000
Mar  9 12:31:53 virtual tvheadend[13221]: CRASH: ??:0 0x7f936f9aa890 0x7f936f998000
Mar  9 12:31:53 virtual tvheadend[13221]: CRASH: ??:0 0x7f936ebdc5a1 0x7f936ea4e000
Mar  9 12:31:53 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/htsmsg.c:373 0x5640c18e7a95 0x5640c16db000
Mar  9 12:31:53 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/webui/webui_api.c:39 (discriminator 3) 0x5640c1955070 0x5640c16db000
Mar  9 12:31:53 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/http.c:1241 0x5640c18bda95 0x5640c16db000
Mar  9 12:31:53 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/http.c:1316 0x5640c18bed3a 0x5640c16db000
Mar  9 12:31:53 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/http.c:1398 0x5640c18bef75 0x5640c16db000
Mar  9 12:31:54 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/http.c:1533 0x5640c18be119 0x5640c16db000
Mar  9 12:31:54 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/http.c:2018 0x5640c18bf2fb 0x5640c16db000
Mar  9 12:31:54 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/http.c:2069 0x5640c18bf619 0x5640c16db000
Mar  9 12:31:54 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/tcp.c:724 0x5640c18b5920 0x5640c16db000
Mar  9 12:31:54 virtual tvheadend[13221]: CRASH: /project/repo/checkout/src/tvh_thread.c:91 0x5640c18b08e8 0x5640c16db000
Mar  9 12:31:54 virtual tvheadend[13221]: CRASH: ??:0 0x7f936f99f6db 0x7f936f998000
Mar  9 12:31:54 virtual tvheadend[13221]: CRASH: clone+0x3f  (/lib/x86_64-linux-gnu/libc.so.6)

History

#1

Updated by Pablo R. over 5 years ago

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
#2

Updated by Pablo R. over 5 years ago

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.

#3

Updated by Flole Systems over 5 years ago

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.

#4

Updated by Pablo R. over 5 years ago

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 */
#5

Updated by Flole Systems over 5 years ago

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.

#6

Updated by Pablo R. over 5 years ago

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

#7

Updated by Jaroslav Kysela over 5 years ago

  • Status changed from New to Fixed
  • % Done changed from 0 to 100

Applied in changeset commit:tvheadend|14d22c3797f2077bc31dfdd03cd1cc5e94511b00.

Also available in: Atom PDF