[vlc-devel] [PATCH 1/2] httpd: mitigate CRSF attack by checking request Origin

[Rémi Denis-Courmont]

Le keskiviikkona 31. tammikuuta 2018, 19.38.01 EET Pierre Lamot a écrit :
> ---
>  src/network/httpd.c | 53
> ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52
> insertions(+), 1 deletion(-)
> 
> diff --git a/src/network/httpd.c b/src/network/httpd.c
> index d9237ebb9c..cfd947ad94 100644
> --- a/src/network/httpd.c
> +++ b/src/network/httpd.c
> @@ -81,6 +81,7 @@ struct httpd_host_t
>      int         *fds;
>      unsigned     nfd;
>      unsigned     port;
> +    char        *psz_origin;
> 
>      vlc_thread_t thread;
>      vlc_mutex_t lock;
> @@ -971,6 +972,7 @@ static httpd_host_t *httpd_HostCreate(vlc_object_t
> *p_this, host->i_client = 0;
>      host->client   = NULL;
>      host->p_tls    = p_tls;
> +    host->psz_origin =  url.psz_host ? strdup( url.psz_host ) : NULL;

I must say, I don't see how that is not going to work in real life.

Normally, the host is not specified when creating a HTTP server - only the 
port number (8080 by default). And even if it is, it might not be the only 
valid origin. To make matters worse, the server instance can be shared by 
other components than the UI - most commonly the HTTP access output.

>      /* create the thread */
>      if (vlc_clone(&host->thread, httpd_HostThread, host,
> @@ -1040,6 +1042,8 @@ void httpd_HostDelete(httpd_host_t *host)
>      net_ListenClose(host->fds);
>      vlc_cond_destroy(&host->wait);
>      vlc_mutex_destroy(&host->lock);
> +    if ( host->psz_origin )
> +        free( host->psz_origin );
>      vlc_object_release(host);
>      vlc_mutex_unlock(&httpd.mutex);
>  }
> @@ -1687,6 +1691,41 @@ auth_failed:
>      return false;
>  }
> 
> +//https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevent
> ion_Cheat_Sheet#Verifying_Same_Origin_with_Standard_Headers
> +static bool
> httpdOriginOk( const httpd_host_t *p_host, const httpd_message_t *p_query )
> +{
> +    char* psz_source_origin = NULL;
> +    char* psz_target_origin = NULL;
> +
> +    const char* psz_origin = NULL;
> +    const char* psz_referer = NULL;
> +    if ( ( psz_origin = httpd_MsgGet( p_query, "Origin") ) != NULL )
> +        psz_source_origin = strdup( psz_origin );
> +    else if ( ( psz_referer = httpd_MsgGet( p_query, "Referer" ) ) != NULL
> ) { +        vlc_url_t referer_url;
> +        vlc_UrlParse( &referer_url, psz_referer );
> +        psz_source_origin = strdup( referer_url.psz_host );
> +        vlc_UrlClean( &referer_url );
> +    } else //unknowned source origin
> +        return true;
> +
> +    const char* psz_host = NULL;
> +    if ( p_host->psz_origin ) {
> +        psz_target_origin = strdup( p_host->psz_origin );
> +    } else if ( ( psz_host = httpd_MsgGet( p_query, "Host" ) ) != NULL ) {
> +        psz_target_origin = strndup( psz_host, strcspn(psz_host, ":") );
> +    } else { //unknowned target origin
> +        free( psz_source_origin );
> +        return true;
> +    }
> +
> +    bool ret = (strcmp( psz_source_origin, psz_target_origin ) == 0);
> +    free( psz_source_origin );
> +    free( psz_target_origin );
> +    return ret;
> +}
> +
> +
>  static void httpdLoop(httpd_host_t *host)
>  {
>      struct pollfd ufd[host->nfd + host->i_client];
> @@ -1821,6 +1860,7 @@ static void httpdLoop(httpd_host_t *host)
>                      default: {
>                          int i_msg = query->i_type;
>                          bool b_auth_failed = false;
> +                        bool b_crsf_failed = false;
> 
>                          /* Search the url and trigger callbacks */
>                          for (int i = 0; i < host->i_url; i++) {
> @@ -1835,8 +1875,17 @@ static void httpdLoop(httpd_host_t *host)
>                                  b_auth_failed = !httpdAuthOk(url->psz_user,
> url->psz_password,
>                                     httpd_MsgGet(query, "Authorization"));
> /* BASIC id */ +
>                                  if (b_auth_failed)
>                                     break;
> +
> +                                if ( query->i_proto == HTTPD_PROTO_HTTP )
> +                                {
> +                                    b_crsf_failed = !httpdOriginOk( host,
> query );
> +
> +                                    if ( b_crsf_failed )
> +                                       break;
> +                                }

This looks like it will break authenticated streaming for no good reasons. No 
thanks.

>                              }
> 
>                              if
> (url->catch[i_msg].cb(url->catch[i_msg].p_sys, cl, answer, query)) @@
> -1858,7 +1907,9 @@ static void httpdLoop(httpd_host_t *host)
>                              answer->i_type   = HTTPD_MSG_ANSWER;
>                              answer->i_version= 0;
> 
> -                           if (b_auth_failed) {
> +                           if ( b_crsf_failed ) {
> +                               answer->i_status = 403;
> +                           } else if ( b_auth_failed ) {
>                                  httpd_MsgAdd(answer, "WWW-Authenticate",
>                                          "Basic realm=\"VLC stream\"");
>                                  answer->i_status = 401;


-- 
雷米‧德尼-库尔蒙
https://www.remlab.net/