0

So I have some code running an IP check to ensure an ADMIN account cannot have access from outside my network.

string strIP = Request.ServerVariables["REMOTE_ADDR"];
if (
    (strIP.Substring(0, 9) != "XXX.XX.X.")
&&  (strIP.Substring(0, 10) != "XXX.XX.XX.")
&&  (strIP.Substring(0, 6) != "XX.XX.")
&&  (strIP.Substring(0, 6) != "XX.XX.")
)
{
..// Check user for being an ADMIN // ....
}

This code was working fine for several weeks, but suddenly has started consistently to err out. The error message is:

Exception

Exception Type: System. ArgumentOUtOfRangeException

Exception Message: Index and length must refer to a location within the string. Parameter name: length.

When I remove the line with "Substring(0,10)", everything works. Also, when I change the line "Substring(0,10)" to "Substring(0,9)" and remove the last ".", everything works.

Can anyone tell me why or perhaps instruct on what is being done incorrectly? For the life of me I can't figure out what is going on.

Analytic Lunatic
  • 3,853
  • 22
  • 78
  • 120
  • What is the length of the strIP? If it is less than 10, it is expected that the strIP.Substring(0, 10) will through an exception. – Tasos K. Oct 25 '13 at 14:49

2 Answers2

1

The problem is that strIP doesn't have 10 characters because your configuration changed for some reason. You could do something like:

(strIP.Length >= 9 && strIP.Substring(0, 9) != "XXX.XX.X.")
||  (strIP.Length >= 10 && strIP.Substring(0, 10) != "XXX.XX.XX.")
||  (strIP.Length >= 6 && strIP.Substring(0, 6) != "XX.XX.")

Notice that the fourth line was a duplicate of the third.

musical_coder
  • 3,886
  • 3
  • 15
  • 18
  • Seems plausible Captain. I shall report back if that solves the issue! – Analytic Lunatic Oct 25 '13 at 15:19
  • @AnalyticLunatic - not saying this answer is wrong, but I feel the switch statement in my answer is more readable and more maintainable, but use whatever works and feels right to you. :-) – Karl Anderson Oct 25 '13 at 15:42
  • @KarlAnderson, I appreciate your input :) For something this small the Switch statement would, by my preference, be a little over the top. Musical_coder had the easiest idea to implement in case we someday have other IP's to include in the criteria to check for. – Analytic Lunatic Oct 25 '13 at 20:28
  • Sorry I didn't think of this before (it's Friday!), but actually, the best way to check if the string represents an IP address is `IPAddress.TryParse()`. See if you can make that work for you. – musical_coder Oct 25 '13 at 20:33
0

Do not allow an out of bounds error to happen by putting a check for length of strIP before you try to do any of the sub-string comparisons, like this:

if (strIP.Length == 10)
{
    if ((strIP.Substring(0, 9) != "XXX.XX.X.")
        &&  (strIP.Substring(0, 10) != "XXX.XX.XX.")
        &&  (strIP.Substring(0, 6) != "XX.XX.")
        &&  (strIP.Substring(0, 6) != "XX.XX."))
    {
        ..// Check user for being an ADMIN // ....
    }
}
else
{
    // Do something here, error, message to user, deny access, etc.
}

UPDATE:

If you want to only apply checks, based upon the length of the string, then use a switch statement, like this:

switch (strIP.Length)
{
    case 6:
        if(strIP.Substring(0, 6) != "XX.XX.")
        {
            // Check user for being an ADMIN
        }
        break;
    case 9:
        if(strIP.Substring(0, 9) != "XXX.XX.X.")
        {
            // Check user for being an ADMIN
        }
        break;
    case 10:
        if(strIP.Substring(0, 10) != "XXX.XX.XX.")
        {
            // Check user for being an ADMIN
        }
        break;
    default:
        // IP string is not in format expected, do something here
        // Most likely would want to err on the side of caution and deny access
        break;
}
Karl Anderson
  • 34,606
  • 12
  • 65
  • 80
  • This would block the entire check from happening unless the string is exactly 10 characters, which doesn't appear to always be the case. – musical_coder Oct 25 '13 at 14:55
  • @musical_coder - well then you cannot AND (`&&`) together your conditions. I was going off of the logic you provided. – Karl Anderson Oct 25 '13 at 14:56
  • @KarlAnderson - I am retrieving the IP address, then checking to ensure that particular substrings of the returned IP do not match my set criteria (preventing ADMIN logon from external locations). I have no way to know that the returned IP string will be 10, or any figure. I suppose if the returned IP is less than 10 that would cause the mentioned line above to throw the error, but how should I go about getting around this problem? – Analytic Lunatic Oct 25 '13 at 14:59