From df63f0e6af5275453a4b6cfa4bc2c9c72093ec4d Mon Sep 17 00:00:00 2001 From: Wesley Miaw Date: Fri, 27 Mar 2020 15:45:23 -0700 Subject: [PATCH] Require CORS Origin header to use https:// and match the entire hostname. Also require the port number to match if specified in the accepted origins list. --- server/dial_server.c | 115 +++++++++++++++++++++++------------- server/main.c | 4 +- server/tests/test_cors.html | 2 +- server/tests/test_cors.sh | 8 +-- 4 files changed, 80 insertions(+), 49 deletions(-) diff --git a/server/dial_server.c b/server/dial_server.c index 48c0ddf..723a18b 100644 --- a/server/dial_server.c +++ b/server/dial_server.c @@ -491,69 +491,100 @@ static void handle_dial_data(struct mg_connection *conn, ds_unlock(ds); } -static int ends_with(const char *str, const char *suffix) { - if (!str || !suffix) + +static int host_matches(const char *origin_host, const char *host) { + if (!origin_host || !host) return 0; - size_t lenstr = strlen(str); - size_t lensuffix = strlen(suffix); - if (lensuffix > lenstr) - return 0; - return strncmp(str + lenstr - lensuffix, suffix, lensuffix) == 0; + + // If the host contains a port number (indicated by a colon) require an + // exact match. + const char * origin_colon = strrchr(origin_host, ':'); + const char * host_colon = strrchr(host, ':'); + if (host_colon != NULL) { + // If the host port number is 443, then accept the origin host if it + // does not have any port number under the assumption we already + // verified https:// + if (strlen(host_colon) == 4 && + strncmp(host_colon, ":443", 4) == 0 && + origin_colon == NULL) + { + // Strip off the host port number and require an exact match. + size_t host_len = host_colon - host; + if (host_len == 0) + return 0; + return strncmp(origin_host, host, host_len) == 0; + } + + // Other port numbers must match exactly. + if (strlen(origin_host) != strlen(host)) + return 0; + return strncmp(origin_host, host, strlen(host)) == 0; + } + + // Otherwise strip off any port number in the origin host, and require an + // exact match. + size_t origin_len = (origin_colon == NULL) ? strlen(origin_host) : origin_colon - origin_host; + if (origin_len == 0) + return 0; + if (origin_len != strlen(host)) + return 0; + return strncmp(origin_host, host, origin_len) == 0; } - // str contains a white space separated list of strings (only supports SPACE characters for now) -static int ends_with_in_list (const char *str, const char *list) { - if (!str || !list) +static int is_hostname_in_list(const char *origin_host, const char *list) { + if (!origin_host || !list) return 0; - const char * scanPointer=list; + const char * scanPointer = list; const char * spacePointer; unsigned int substringSize = 257; - char *substring = (char *)malloc(substringSize); - if (!substring){ + char *host = (char *)malloc(substringSize); + if (!host) { return 0; } - while ( (spacePointer =strchr(scanPointer, ' ')) != NULL) { + while ((spacePointer = strchr(scanPointer, ' ')) != NULL) { int copyLength = spacePointer - scanPointer; - // protect against buffer overflow - if (copyLength>=substringSize){ - substringSize=copyLength+1; - free(substring); - substring=(char *)malloc(substringSize); - if (!substring){ - return 0; - } - } + // protect against buffer overflow + if (copyLength >= substringSize){ + substringSize = copyLength + 1; + free(host); + host = (char *)malloc(substringSize); + if (!host) { + return 0; + } + } - memcpy(substring, scanPointer, copyLength); - substring[copyLength] = '\0'; - //printf("found %s \n", substring); - if (ends_with(str, substring)) { - free(substring); substring = NULL; + memcpy(host, scanPointer, copyLength); + host[copyLength] = '\0'; + //printf("found %s \n", host); + if (host_matches(origin_host, host)) { + free(host); host = NULL; return 1; } scanPointer = scanPointer + copyLength + 1; // assumption: only 1 character } - free(substring); substring = NULL; - return ends_with(str, scanPointer); -} - -static int should_check_for_origin( char * origin ) { - const char * const CHECK_PROTOS[] = { "http:", "https:", "file:" }; - for (int i = 0; i < 3; ++i) { - if (!strncmp(origin, CHECK_PROTOS[i], strlen(CHECK_PROTOS[i]) - 1)) { - return 1; - } - } - return 0; + free(host); host = NULL; + return host_matches(origin_host, scanPointer); } static int is_allowed_origin(DIALServer* ds, char * origin, const char * app_name) { - if (!origin || strlen(origin)==0 || !should_check_for_origin(origin)) { + const char * const HTTPS_PROTO = "https://"; + fprintf(stderr, "checking %s for %s\n", origin, app_name); + + if (!origin || strlen(origin)==0) { return 1; } + + // Make sure the origin begins with HTTPS. + if (strncmp(origin, HTTPS_PROTO, strlen(HTTPS_PROTO)) != 0) { + return 0; + } + + // For the rest of the check, we only care about the hostname and optional + // port number. + const char *origin_host = origin + strlen(HTTPS_PROTO); if (!ds_lock(ds)) { // If we can't check, fail in favor of safety. @@ -564,7 +595,7 @@ static int is_allowed_origin(DIALServer* ds, char * origin, const char * app_nam for (app = ds->apps; app != NULL; app = app->next) { if (!strcmp(app->name, app_name)) { if (!app->corsAllowedOrigin[0] || - ends_with_in_list(origin, app->corsAllowedOrigin)) { + is_hostname_in_list(origin_host, app->corsAllowedOrigin)) { result = 1; break; } diff --git a/server/main.c b/server/main.c index d515c1c..1019531 100644 --- a/server/main.c +++ b/server/main.c @@ -288,8 +288,8 @@ void runDial(void) struct DIALAppCallbacks cb_yt = {youtube_start, youtube_hide, youtube_stop, youtube_status}; struct DIALAppCallbacks cb_system = {system_start, system_hide, NULL, system_status}; - if (DIAL_register_app(ds, "Netflix", &cb_nf, NULL, 1, ".netflix.com") == -1 || - DIAL_register_app(ds, "YouTube", &cb_yt, NULL, 1, ".youtube.com") == -1 || + if (DIAL_register_app(ds, "Netflix", &cb_nf, NULL, 1, "netflix.com www.netflix.com port.netflix.com:123") == -1 || + DIAL_register_app(ds, "YouTube", &cb_yt, NULL, 1, "youtube.com www.youtube.com port.youtube.com:123") == -1 || DIAL_register_app(ds, "system", &cb_system, NULL, 1, "") == -1) { printf("Unable to register DIAL applications.\n"); diff --git a/server/tests/test_cors.html b/server/tests/test_cors.html index 451a96e..f3d651b 100644 --- a/server/tests/test_cors.html +++ b/server/tests/test_cors.html @@ -17,7 +17,7 @@ and open the template in the editor. console.log("make dial post..."); var ip = $("#ipAddress").val(); var port = $("#dialPort").val(); - var urlStr = "http://"+ip+":"+port+"/apps/"+app; + var urlStr = "https://"+ip+":"+port+"/apps/"+app; console.log(urlStr); $("#status").text("posted to "+urlStr); $.ajax({ diff --git a/server/tests/test_cors.sh b/server/tests/test_cors.sh index 088fe02..c594bed 100755 --- a/server/tests/test_cors.sh +++ b/server/tests/test_cors.sh @@ -9,7 +9,7 @@ ip_address=$1 port=$2 #Testing all the positive cases -origins="http://www4.netflix.com http://1.netflix.com https://www.netflix.com https://www4.netflix.com ftp://this.is.fine" +origins="https://www.netflix.com https://netflix.com https://port.netflix.com:123 https://www.netflix.com:80 https://www.netflix.com:123" for origin in $origins; do curl --fail --silent --header "Origin:$origin" --data "v=QH2-TGUlwu4" http://$ip_address:$port/apps/Netflix || echo "failed: $origin should be accepted" curl --fail --silent --header "Origin:$origin" -X OPTIONS http://$ip_address:$port/apps/Netflix || echo "failed: $origin should be accepted" @@ -21,7 +21,7 @@ then fi done -origins="http://www4.youtube.com http://1.youtube.com https://www.youtube.com https://www4.youtube.com ftp://this.is.fine" +origins="https://www.youtube.com https://youtube.com https://port.youtube.com:123 https://www.youtube.com:80 https://www.youtube.com:123" for origin in $origins; do curl --fail --silent --header "Origin:$origin" --data "v=QH2-TGUlwu4" http://$ip_address:$port/apps/YouTube || echo "failed: $origin should be accepted" curl --fail --silent --header "Origin:$origin" -X OPTIONS http://$ip_address:$port/apps/YouTube || echo "failed: $origin should be accepted" @@ -34,7 +34,7 @@ fi done #Testing all the negative cases -origins="http://www.netflix-a.com http://www.netflix.com4 http://a-netflix.com https://ww.netflix-a.com https://www.netflix.com4 https://a-netflix.com http://netflix.com http://www.attack.com https://www.attack.com file://www.attack.com" +origins="http://www.netflix-a.com http://www.netflix.com4 http://a-netflix.com http://www4.netflix.com https://port.netflix.com:1234 http://1.netflix.com https://www4.netflix.com https://ww.netflix-a.com https://www.netflix.com4 https://a-netflix.com http://netflix.com http://www.attack.com https://www.attack.com file://www.attack.com ftp://this.is.not.fine" for origin in $origins; do curl --fail --silent --header "Origin:$origin" --data "v=QH2-TGUlwu4" http://$ip_address:$port/apps/Netflix && echo "failed: $origin should be rejected" curl --fail --silent --header "Origin:$origin" -X OPTIONS http://$ip_address:$port/apps/Netflix && echo "failed: $origin should be rejected" @@ -46,7 +46,7 @@ then fi done -origins="http://www.youtube-a.com http://www.youtube.com4 http://a-youtube.com https://ww.youtube-a.com https://www.youtube.com4 https://a-youtube.com http://youtube.com https://youtube.com http://www.attack.com https://www.attack.com file://www.attack.com" +origins="http://www.youtube-a.com http://www.youtube.com4 http://a-youtube.com https://ww.youtube-a.com http://www4.youtube.com https://port.youtube.com:1234 http://1.youtube.com https://www4.youtube.com https://www.youtube.com4 https://a-youtube.com http://youtube.com http://www.attack.com https://www.attack.com file://www.attack.com ftp://this.is.not.fine" for origin in $origins; do curl --fail --silent --header "Origin:$origin" --data "v=QH2-TGUlwu4" http://$ip_address:$port/apps/YouTube && echo "failed: $origin should be rejected" curl --fail --silent --header "Origin:$origin" -X OPTIONS http://$ip_address:$port/apps/YouTube && echo "failed: $origin should be rejected"