wet-boew / web-reporting

1 stars 2 forks source link

The application allows Cross-site scripting (Javascript code injection) that intefere with the page #45

Closed LaurentGoderre closed 11 years ago

LaurentGoderre commented 11 years ago

@dfait-webstandards lets discuss this offline

FarazAliKhan commented 11 years ago

Hello,

As you mentioned us two ways to fix this issue. We implemented a first one but not second one. We are using query string for language passing between the pages. If I will use your strategy to fix this issue it will takes time because all pages using query string for language passing between the pages. We already mentioned to TBS in the beginning of this project that we are using query string only for language passing because at that time TBS had a some concerned with query string. We are only passing language string not IDS such as institution or reporting ID. so, I think it will not be the big issue please let me know about your thoughts.

Thanks, Faraz

FarazAliKhan commented 11 years ago

I am not understand your point to get language from HTML (JavaScript ) because we always set language variable via code behind. Anyways I understand your concern so, I will set a Session variable in the application and I hope it will resolve this issue.

Please let me know if you have any question.

FarazAliKhan commented 11 years ago

I am not understand your point to get language from HTML (JavaScript ) because we always set language variable via code behind. Anyways I understand your concern so, I will set a language Session variable in the application and I hope it will resolve this issue.

Please let me know if you have any question.

LaurentGoderre commented 11 years ago

You don't need to change the .NET, just the Javascript. Here is how it would be instead

<script language="javascript" type="text/javascript">
        var sessionTimeoutWarning = "110";
        var sessionTimeout = "120";
        var timeOnPageLoad = new Date();
        var lang=PE.language;
        //For warning
        setTimeout('SessionWarning()', parseInt(sessionTimeoutWarning) * 60 * 1000);
        //To redirect to the welcome page
        setTimeout('RedirectToWelcomePage()', parseInt(sessionTimeout) * 60 * 1000);

        //Session Warning
        function SessionWarning() {
            //minutes left for expiry
             var message=null;

            var minutesForExpiry = (parseInt(sessionTimeout) - parseInt(sessionTimeoutWarning));
            if (lang == "fr") {
             message = "• Votre séance expirera dans " + minutesForExpiry + " minutes. Veuillez sauvegarder vos données avant l’expiration de la séance.";
            }
            else
            {
             message = "Your session will expire in another " + minutesForExpiry + " minutes. Please save the data before the session expires.";

            }
            alert(message);
            var currentTime = new Date();
            //time for expiry
            var timeForExpiry = timeOnPageLoad.setMinutes(timeOnPageLoad.getMinutes() + parseInt(sessionTimeout));

            //Current time is greater than the expiry time
            if (Date.parse(currentTime) > timeForExpiry) {
                if (lang == "fr") {
                    alert("Cette séance est expirée. Vous serez redirigé vers la page d’ouverture de séance.");
                }
                else {
                    alert("This session has expired. You will be redirected to the login page.");
                }
                window.location = "/ws-nw/wr-rw/index.aspx";
            }
        }

        //Session timeout
        function RedirectToWelcomePage() {
            if (lang == "fr") {
                alert("Cette séance est expirée. Vous serez redirigé vers la page d’ouverture de séance.");
            }
            else {
                alert("This session has expired. You will be redirected to the login page.");
            }

            window.location = "/ws-nw/wr-rw/index.aspx";
        }
    </script>
FarazAliKhan commented 11 years ago

Can we chat over the phone?

FarazAliKhan commented 11 years ago

var lang=PE.language // how do I get the value.

In all cases I have to add the code so I will go with language session approach. I will fix this issue over the weekend

LaurentGoderre commented 11 years ago

PE.language will give you the value you need. Changing from the Querystring makes me nervous because it means a lot more testing required.

FarazAliKhan commented 11 years ago

We need session or query string or any framework to fill this "lang" variable from HTML for example Example: var lang="<%= Session["Lang"] %>" or var lang=Request.QueryString["lang"]; PE.language will not get any value directly until unless you have a framework.

If you have a perfect example to fix this issue with your strategy you can send me a code.

Changing from the Querystring it seems to me allot work also because we need to change Master pages also.

For now I will focus on other issues once I will finish other issues then we will discuss on that more.

LaurentGoderre commented 11 years ago

Your Master page already correctly identifies the page language. plus the application uses the Web Experience Toolkit which has a way to detect the language in a safe way (PE.language). The only change that will need to happen on your script is using en and fr instead of eng and fra

var lang=PE.language;
LaurentGoderre commented 11 years ago

BTW, I have tested this and it works. You can open a javascript console on any page and run PE.language and you will get the language of the page.

FarazAliKhan commented 11 years ago

Ok send me a example .

FarazAliKhan commented 11 years ago

Ok if possible can you please send me a code or example?

LaurentGoderre commented 11 years ago

This code should work:

<script language="javascript" type="text/javascript">
        var sessionTimeoutWarning = "110";
        var sessionTimeout = "120";
        var timeOnPageLoad = new Date();
        var lang=PE.language;
        //For warning
        setTimeout('SessionWarning()', parseInt(sessionTimeoutWarning) * 60 * 1000);
        //To redirect to the welcome page
        setTimeout('RedirectToWelcomePage()', parseInt(sessionTimeout) * 60 * 1000);

        //Session Warning
        function SessionWarning() {
            //minutes left for expiry
             var message=null;

            var minutesForExpiry = (parseInt(sessionTimeout) - parseInt(sessionTimeoutWarning));
            if (lang == "fr") {
             message = "• Votre séance expirera dans " + minutesForExpiry + " minutes. Veuillez sauvegarder vos données avant l’expiration de la séance.";
            }
            else
            {
             message = "Your session will expire in another " + minutesForExpiry + " minutes. Please save the data before the session expires.";

            }
            alert(message);
            var currentTime = new Date();
            //time for expiry
            var timeForExpiry = timeOnPageLoad.setMinutes(timeOnPageLoad.getMinutes() + parseInt(sessionTimeout));

            //Current time is greater than the expiry time
            if (Date.parse(currentTime) > timeForExpiry) {
                if (lang == "fr") {
                    alert("Cette séance est expirée. Vous serez redirigé vers la page d’ouverture de séance.");
                }
                else {
                    alert("This session has expired. You will be redirected to the login page.");
                }
                window.location = "/ws-nw/wr-rw/index.aspx";
            }
        }

        //Session timeout
        function RedirectToWelcomePage() {
            if (lang == "fr") {
                alert("Cette séance est expirée. Vous serez redirigé vers la page d’ouverture de séance.");
            }
            else {
                alert("This session has expired. You will be redirected to the login page.");
            }

            window.location = "/ws-nw/wr-rw/index.aspx";
        }
    </script>
FarazAliKhan commented 11 years ago

right now this code already in 1ColSubsiteBase.master page when I will set var lang=PE.language; instead of var lang="<%= Session["Lang"] %>". How do I access in .cs page?

LaurentGoderre commented 11 years ago

You would not change the way you do it in .cs files. Basically if someone tries to mess with the ?lang=eng, it will revert to the neutral language, english.

FarazAliKhan commented 11 years ago

If you discuss regarding user handling not security. Here it is some test cases below:

FYI, It will revert to the neutral language without any new code added. This revert functionality is already implemented in the production version.

Test#1:

URL: http://localhost:11027/IIS/administration/institution.aspx?lang=eng

1) if user remove lang=eng and URL looks like that (http://localhost:11027/IIS/administration/institution.aspx?) then browser URL automatically set a http://localhost:11027/IIS/administration/institution.aspx?lang=eng

2) if user remove =eng and URL looks like that http://localhost:11027/IIS/administration/institution.aspx?lang then browser URL automatically set a http://localhost:11027/IIS/administration/institution.aspx?lang=eng

3) if user remove eng and URL looks like that http://localhost:11027/IIS/administration/institution.aspx?lang= then browser will not set URL automatically and warning will come out "The page isn't redirecting properly" (it is not a error browser is working like that).

I tested these scenario with current version and added you code var lang=PE.language; into 1ColSubsiteBase.master and the results are same.

Please let me know should I added var lang=PE.language; into 1ColSubsiteBase.master or remain as is?

LaurentGoderre commented 11 years ago

Yes you update your script for the session timeout with the one I provided

FarazAliKhan commented 11 years ago

Perfect.

Thanks, Faraz

FarazAliKhan commented 11 years ago

Untitled1

Please attached image. I have added your code into 1ColSubsiteBase.master but var lang=PE.language; is not working properly that's why I put my code var lang="<%= Session["Lang"]%>". My code is already in prod code also (No new changes).

FarazAliKhan commented 11 years ago

I used this line of code.

LaurentGoderre commented 11 years ago

This is still happening in the January release

LaurentGoderre commented 11 years ago

I just realized, since it's in the body and it's using an outdated version of wet, the code needs to be wrapped in a document.ready. The following code should work:

<script language="javascript" type="text/javascript">
    $(document).ready(function(){
        var sessionTimeoutWarning = "110";
        var sessionTimeout = "120";
        var timeOnPageLoad = new Date();
        var lang=PE.language;
        //For warning
        setTimeout('SessionWarning()', parseInt(sessionTimeoutWarning) * 60 * 1000);
        //To redirect to the welcome page
        setTimeout('RedirectToWelcomePage()', parseInt(sessionTimeout) * 60 * 1000);

        //Session Warning
        function SessionWarning() {
            //minutes left for expiry
             var message=null;

            var minutesForExpiry = (parseInt(sessionTimeout) - parseInt(sessionTimeoutWarning));
            if (lang == "fr") {
             message = "• Votre séance expirera dans " + minutesForExpiry + " minutes. Veuillez sauvegarder vos données avant l’expiration de la séance.";
            }
            else
            {
             message = "Your session will expire in another " + minutesForExpiry + " minutes. Please save the data before the session expires.";

            }
            alert(message);
            var currentTime = new Date();
            //time for expiry
            var timeForExpiry = timeOnPageLoad.setMinutes(timeOnPageLoad.getMinutes() + parseInt(sessionTimeout));

            //Current time is greater than the expiry time
            if (Date.parse(currentTime) > timeForExpiry) {
                if (lang == "fr") {
                    alert("Cette séance est expirée. Vous serez redirigé vers la page d’ouverture de séance.");
                }
                else {
                    alert("This session has expired. You will be redirected to the login page.");
                }
                window.location = "/ws-nw/wr-rw/index.aspx";
            }
        }

        //Session timeout
        function RedirectToWelcomePage() {
            if (lang == "fr") {
                alert("Cette séance est expirée. Vous serez redirigé vers la page d’ouverture de séance.");
            }
            else {
                alert("This session has expired. You will be redirected to the login page.");
            }

            window.location = "/ws-nw/wr-rw/index.aspx";
        }
    }
</script>
FarazAliKhan commented 11 years ago

Ok I will add this line of code and let you know by tomorrow.

FarazAliKhan commented 11 years ago

I tried your code but no luck even no message display.

FYI, above code will only display warning and session time out message. Let say If we will change or modify JS code in the master page (1ColSubsiteBase.master) so, there is no impact on the application and your concern will still there because it is only display session timeout warning message. We have only one way to change Request.QueryString["lang"] into Session of each page of application and add Splash page in the starting of application for select English and French.

Please let me know if you have any question....

FarazAliKhan commented 11 years ago

Also we need to change a some logics in master pages. It will be the big change but this is the only way.

LaurentGoderre commented 11 years ago

The problem is using the value of the querystring which can be used to inject code.

FarazAliKhan commented 11 years ago

Yes but wars app is running on intranet not internet and we just passing only lang id (no other ids such as institution, reporting id via query string). If you think this will be very major issue so, we can convert into sessions and remove query string from url.

According to my point of view if application on intranet based (behind the couple of firewalls) and implemented all other security features so, there is no issue with query string if you are only passing lang variable. The query string is dangerous when you are passing IDs either encrypted or not encrypted.

LaurentGoderre commented 11 years ago

The querystring is dangerous when it's data is used unfiltered. This could lead to phishing or other malicious attacks

FarazAliKhan commented 11 years ago

Yes but wars app is running on intranet not internet and we just passing only lang variable (no other ids such as institution, reporting id via query string). If you think this will be very major issue so, we can convert into sessions and remove query string from url.

According to my point of view if application on intranet based (behind the couple of firewalls) and implemented all other security features so, there is no issue with query string if you are only passing lang variable. The query string is dangerous when you are passing IDs either encrypted or not encrypted.

LaurentGoderre commented 11 years ago

I just tested this code and it works

<asp:Content ContentPlaceHolderID="MainContentPlaceHolder" runat="server">
    <asp:ContentPlaceHolder ID="MainContentPlaceHolder" runat="server" ></asp:ContentPlaceHolder>
    <script language="javascript" type="text/javascript">
        var sessionTimeoutWarning = "<%= System.Configuration.ConfigurationSettings.AppSettings["SessionWarning"].ToString()%>";
        var sessionTimeout = "<%= Session.Timeout %>";
        var timeOnPageLoad = new Date();
        var lang=PE.language;

        //For warning
        setTimeout(SessionWarning, parseInt(sessionTimeoutWarning) * 60 * 1000);
        //To redirect to the welcome page
        setTimeout(RedirectToWelcomePage, parseInt(sessionTimeout) * 60 * 1000);

        //Session Warning
        function SessionWarning() {
            //minutes left for expiry
             var message=null;

            var minutesForExpiry = (parseInt(sessionTimeout) - parseInt(sessionTimeoutWarning));
            if (lang == "fr") {
             message = "• Votre séance expirera dans " + minutesForExpiry + " minutes. Veuillez sauvegarder vos données avant l’expiration de la séance.";
            }
            else
            {
             message = "Your session will expire in another " + minutesForExpiry + " minutes. Please save the data before the session expires.";
            }
            alert(message);
        }

        //Session timeout
        function RedirectToWelcomePage() {
            if (lang == "fr") {
                alert("Cette séance est expirée. Vous serez redirigé vers la page d’ouverture de séance.");
            }
            else {
                alert("This session has expired. You will be redirected to the login page.");
            }

            window.location = "/ws-nw/wr-rw/index.aspx";
        }
    </script>
</asp:Content>
FarazAliKhan commented 11 years ago

is it showing a session warning and timeout message?

LaurentGoderre commented 11 years ago

Yep

FarazAliKhan commented 11 years ago

Laurent the problem is not with this code. sometimes you are talking about query string and sometime talking about this code. I am unable to understand your concerns. If you want stop javascript injection then we have to remove query string from the URL.

if will we set this code then issue#45 will be resolve or not?