[Hamlib-developer] rigctld remote exploitation without an open port

[Matt P. <mh...@mi...>]

I'm following the Hamlib Security Policy on GitHub to report this
here. It has my iffy attempt at a patch, and some discussion of the
security problem (which might be non-obvious).

The rigctld man page says "DO NOT leave this TCP port open wide to the
Internet." However, even if there's no connectivity on port 4532 from
the Internet to the computer running rigctld, there's still a risk if
that computer is ever used for web browsing (even with the latest
version of Chrome). An external https web page can use the JavaScript
fetch API to connect to TCP port 4532 on 127.0.0.1, and send a rigctl
command, e.g., a URL ending with /F_28074000 will change the frequency
to 28.074. At least for me, it's 100% reproducible.

It's possible to change rigctld to block this. Although the likelihood
of exploitation may be small, the impact may be catastrophic in some
cases. First, there might be a remote compromise of the computer. As
far as I can tell, the rigctl parser isn't yet robust enough for
arbitrary command input originating from untrusted websites, e.g., the
various unbounded fscanf calls. Second, there may be effects on the
station. The frequency might be surreptitiously moved to one not
allowed by a license, or with a transmitter/antenna mismatch that
damages equipment or conceivably starts a fire. Even if the station
was carefully designed, it still may be possible for rigctld to make
changes more quickly than the operator can react.

For example:
  - Someone compromises a DXpedition website during the expedition, when
    it has many visitors per hour (happened about 6 weeks ago, for example).

  - They insert JavaScript code that automatically causes every visitor's
    web browser to try to send unwanted rigctl commands to 127.0.0.1 port
    4532 (perhaps hasn't happened yet, but is easy, and only a few
    less-used web browsers defend against this).

  - A small percentage of visitors are running rigctld and would face
    either inconvenience or a significant station problem.

I don't know the best way to change rigctld, but I worked with three ideas:
  - It's only necessary to block exactly the type of network traffic that can
    be sent by web browsers, and this is very constrained.

  - The rigctl parser consumes the input stream, often a character at a time,
    and this might make it difficult to apply a set of rules about good input
    versus bad, or to make the changes in rigctld.c instead of the parser.

  - Because exploitation attempts would be uncommon, and avoidable by moving
    off the malicious website, it may be acceptable to shut down rigctld and
    warn the user (i.e., not bother to leave the good input uninterrupted).

(I realize that browsers can't send raw spaces in a URL such as
"F 28074000" and this may have led to a different solution approach if
rigctld doesn't need to support F_28074000 as well.)

(Some web browsers have Private Network Access that solves a different
problem with remote connections to 127.0.0.1, but this doesn't help
rigctld, which executes rigctl commands found in an OPTIONS request.)

More specifically, the approach below tries to:
  - Find the beginning of the browser-sent data (such as "GET /") by looking
    for only two characters (such as "GE"). This helps to ensure the problem
    is detected before rigctld can process the current rigctl command.

  - Recognize that the "/" in "GET /" will always be detected by the parser
    as an invalid command. Detecting at that point is not always soon enough
    to prevent rigctld from processing the current rigctl command.

  - Only disrupt traffic from web browsers to rigctld, and not interfere with
    any other possible integration (nc commands, etc.).

  - Offer a way to turn off this feature if it breaks someone's use case.

  - Also limit string fscanf calls to 30 or 511 characters (not unbounded).

ampctld and rotctld could be updated analogously.

There are much cleaner approaches but they may need deeper code changes.

--- rigctl_parse.c.old	2024-10-13 02:29:04.929230005 +0000
+++ rigctl_parse.c	2024-10-13 02:59:54.250525125 +0000
@@ -82,6 +82,7 @@
 #define STR(S) STR1(S)
 
 #define MAXNAMSIZ 32
+#define MAXFSCANFSIZ 30
 #define MAXNBOPT 100    /* max number of different options */
 #define MAXARGSZ 511
 
@@ -568,7 +569,14 @@
         *(char *)p = 0;
 
 
-        ret = fscanf(fin, format, p);
+        if (!strcmp(format, "%s"))
+        {
+            ret = fscanf(fin, "%" STR(MAXARGSZ) "s", (char *) p);
+        }
+        else
+        {
+            ret = fscanf(fin, format, p);
+        }
 
         if (ret < 0)
         {
@@ -819,7 +827,8 @@
                         return (RIGCTL_PARSE_ERROR);
                     }
 
-                    retcode = fscanf(fin, "%s", ++pcmd);
+                    ++pcmd;
+                    retcode = fscanf(fin, "%" STR(MAXFSCANFSIZ) "s", pcmd);
 
                     if (retcode == 0) { rig_debug(RIG_DEBUG_WARN, "%s: unable to scan %c\n", __func__, *(pcmd - 1)); }
 
@@ -915,6 +924,22 @@
 
         if (!cmd_entry)
         {
+            if (cmd == '/')
+            {
+                if (!getenv("HAMLIB_IGNORE_CTLD_HTTP"))
+                {
+                    fprintf(stderr, "A '/' command, which is not valid in Hamlib, was detected. If this\n");
+                    fprintf(stderr, "occurred during use of any 'ctld' application (ampctld, rigctld, or\n");
+                    fprintf(stderr, "rotctld), it may mean that a Hamlib request was sent by a website,\n");
+                    fprintf(stderr, "presenting a security risk to your computer and station. If that is not\n");
+                    fprintf(stderr, "applicable, or if you intentionally use a web browser to access ctld\n");
+                    fprintf(stderr, "applications, then you can restart any Hamlib application after setting\n");
+                    fprintf(stderr, "the HAMLIB_IGNORE_CTLD_HTTP environment variable. It can be safer\n");
+                    fprintf(stderr, "not to run a web browser (that allows Hamlib requests to be embedded in\n");
+                    fprintf(stderr, "external websites) on the same computer as any ctld application.\n");
+                    exit(1);
+                }
+            }
             if (cmd != ' ')
             {
                 fprintf(stderr, "Command '%c' not found!\n", cmd);
@@ -1035,6 +1060,39 @@
                 arg1[0] = fgetc(fin);
                 arg1[1] = 0;
 
+                if (!getenv("HAMLIB_IGNORE_CTLD_HTTP"))
+                {
+                    char start_of_http_method[3];
+
+                    start_of_http_method[0] = cmd;
+                    start_of_http_method[1] = arg1[0];
+                    start_of_http_method[2] = '\0';
+                    /* Modern web browsers can only use the GET, HEAD, OPTIONS, and POST methods
+                     * unless the web server sends an Access-Control-Allow-Methods HTTP response
+                     * header (the Hamlib 'ctld' programs are not web servers and, of course,
+                     * do not send that header). If HTTP traffic is sent, then the first two
+                     * letters of the method name are available at this point in Hamlib's parsing.
+                     */
+                    if (!strcmp(start_of_http_method, "GE") ||
+                       !strcmp(start_of_http_method, "HE") ||
+                       !strcmp(start_of_http_method, "OP") ||
+                       !strcmp(start_of_http_method, "PO"))
+                    {
+                        fprintf(stderr, "The '%s' command, which is not valid in Hamlib, was detected. If this\n",
+                           start_of_http_method);
+                        fprintf(stderr, "occurred during use of any 'ctld' application (ampctld, rigctld, or\n");
+                        fprintf(stderr, "rotctld), it may mean that a Hamlib request was sent by a website,\n");
+                        fprintf(stderr, "presenting a security risk to your computer and station. If that is not\n");
+                        fprintf(stderr, "applicable, or if you intentionally use a web browser to access ctld\n");
+                        fprintf(stderr, "applications, then you can restart any Hamlib application after setting\n");
+                        fprintf(stderr, "the HAMLIB_IGNORE_CTLD_HTTP environment variable. It can be safer\n");
+                        fprintf(stderr, "not to run a web browser (that allows Hamlib requests to be embedded in\n");
+                        fprintf(stderr, "external websites) on the same computer as any ctld application.\n");
+                        exit(1);
+                    }
+                }
+                
+
                 if (debugflow) { rig_debug(RIG_DEBUG_TRACE, "%s: debug4 arg1=%c\n", __func__, arg1[0]); }
 
                 if (prompt && arg1[0] == 0x0a)
@@ -4037,7 +4095,7 @@
     const chan_t *chan_list;
     channel_t chan;
     int status;
-    char s[16];
+    char s[MAXARGSZ + 1];
 
     ENTERFUNC2;
 
Matt, KA1R