Bug in 1.2.15 final

Topics: Developer Forum, User Forum
Oct 23, 2008 at 6:02 PM
Sorry to find this right after the release...

on line #2149 the following code appears

strcpy_s(pOut,INTERNET_MAX_URL_LENGTH-(

int)(pOut-outString), Value);
pOut+= strlen(Value);
*pOut=
'\0';

when strcpy_s fails(and it does here) the pointer should not be incremented.
A simple fix is 

if(strcpy_s(pOut,INTERNET_MAX_URL_LENGTH-(int)(pOut-outString), Value)==0){
    pOut+= strlen(Value);
    *pOut=
'\0';
}
This also triggers the invalid parameter handler.  A better fix would be to check the string length before the copy.

To duplicate the error use the following INI

 

 

# get rid of instmsg/aliases
RewriteCond %{HTTP_URL}             ^(.*)/instmsg/aliases$  [NF]
RewriteRule ^(.+)$                  $1                        [NF,L]

and the use a really long URL http://127.0.0.1/test/test.aspx?blahblah......
The URL that was hitting me was 2012 characters long. 
Why such a long URL?  A combination of script kiddies and bad code on my part.

Fred

Coordinator
Oct 24, 2008 at 2:25 AM
Fred, thanks for reporting this.
I created work item 19137 for this. http://www.codeplex.com/IIRF/WorkItem/View.aspx?WorkItemId=19137

I couldn't reproduce the problem you described with a long URL and the rules you provided.
But code inspection shows you are correct, so I updated the code anyway.

In testing your rule, I also found that 404's [NF] were not working. 
So I created a work item for that problem, too.http://www.codeplex.com/IIRF/WorkItem/View.aspx?WorkItemId=19135  

And also, I found an optimization to make when [NF] or [F] is used on a rule: skip generation of the replacement string, which is never used. 
http://www.codeplex.com/IIRF/WorkItem/View.aspx?WorkItemId=19136

All these workitems are fixed in change set 42215.
They are all included in the R2 release of v1.2.15, available on the releases tab right now.

Oct 24, 2008 at 4:45 PM
sorry, I thought I gave you a good test case. Lets try again:
---isapirewrite4.ini---
RewriteLog C:\www\isapirewrite.Log
RewriteLogLevel 4
IterationLimit 10
# get rid of instmsg/aliases
RewriteCond %{HTTP_URL} ^(.*)/instmsg/aliases$ [NF]
RewriteRule ^(.+)$ $1 [NF,L]
--url---
http://127.0.0.1/test/test.aspx/test/43332003/Testbin/57645353/testitemid/306/testgroup/test.aspx/act/catalog.cfm/test/43332003/Testbin/57645353/category/Power%20and%20Squat%20Racks/browse/null/testgroup/test.aspx/test/43332003/Testbin/57645353/testitemid/306/testgroup/test.aspx/act/catalog.cfm/test/43332003/Testbin/57645353/category/Power%20and%20Squat%20Racks/browse/null/testgroup/test.aspx/act/Catalog.cfm/test/43332003/Testbin/57645353/catalogid/545/category/Power%20and%20Squat%20Racks/browse/null/testgroup/test.aspx/desc/test.aspx/act/Catalog.cfm/test/43332034/Testbin/88590455/catalogid/976/description/null/browse/null/testgroup/Home/desc/test.aspx/act/Catalog.cfm/test/43332125/Testbin/81150674/catalogid/665/description/null/browse/null/testgroup/Home/desc/test.aspx/act/Catalog.cfm/test/43332303/Testbin/94764803/catalogid/758/description/null/browse/null/testgroup/Home/desc/test.aspx/act/Catalog.cfm/test/43332342/Testbin/988059/catalogid/63/description/null/browse/null/testgroup/Home/test.aspx/test/43332003/Testbin/57645353/testitemid/306/testgroup/test.aspx/act/catalog.cfm/test/43332003/Testbin/57645353/category/Power%20and%20Squat%20Racks/browse/null/testgroup/test.aspx/test/43332003/Testbin/57645353/testitemid/306/testgroup/test.aspx/act/catalog.cfm/test/43332003/Testbin/57645353/category/Power%20and%20Squat%20Racks/browse/null/testgroup/test.aspx/act/Catalog.cfm/test/43332003/Testbin/57645353/catalogid/545/category/Power%20and%20Squat%20Racks/browse/null/testgroup/test.aspx/desc/test.aspx/act/Catalog.cfm/test/43332034/Testbin/88590455/catalogid/976/description/null/browse/null/testgroup/Home/desc/test.aspx/act/Catalog.cfm/test/43332125/Testbin/81150674/catalogid/665/description/null/browse/null/testgroup/Home/desc/test.aspx/act/Catalog.cfm/test/43332303/Testbin/94764803/catalogid/758/description/null/browse/null/testgroup/Home/desc/test.aspx/act/Catalog.cfm/test/43332342/Testbin/988059/catalogid/63/description/null/browse/null/testgroup/Home/desc/test.aspx?act=catalog.cfm&test=43332003&Testbin=57645353&category=Power%20and%20Squat%20Racks&startrow=1&browse=&testgroup=test.aspx&at=theend
Note that 127.0.0.1/test must be changed to a vaild virtual directory. You can copy and paste this url to firefox(3.0.1) to recreate the error. IE chops the url off and does not recreate the error.
I look forward to trying your latest release this weekend on a production server.
Fred
----- Original Message -----
From: [email removed]
To: [email removed]
Sent: Thursday, October 23, 2008 9:25 PM
Subject: Re: Bug in 1.2.15 final [IIRF:38370]

From: Cheeso

Fred, thanks for reporting this.
I created work item 19137 for this. http://www.codeplex.com/IIRF/WorkItem/View.aspx?WorkItemId=19137

I couldn't reproduce the problem you described with a long URL and the rules you provided.
But code inspection shows you are correct, so I updated the code anyway.

In testing your rule, I also found that 404's [NF] were not working.
So I created a work item for that problem, too.http://www.codeplex.com/IIRF/WorkItem/View.aspx?WorkItemId=19135

And also, I found an optimization to make when [NF] or [F] is used on a rule: skip generation of the replacement string, which is never used.
http://www.codeplex.com/IIRF/WorkItem/View.aspx?WorkItemId=19136

All these workitems are fixed in change set 42215.
They are all included in the R2 release of v1.2.15, available on the releases tab right now.

Oct 24, 2008 at 6:54 PM
and to reply to my own post:
I had tested in 1.2.14. Today I tested in 1.2.15R2.
I fixed the ini file as follows:

RewriteLog  C:\www\isapirewrite.Log
RewriteLogLevel 5
IterationLimit 10

# get rid of instmsg/aliases
RewriteCond %{HTTP_URL}             ^(.*)/instmsg/aliases$
RewriteRule ^(.+)$                  $1                        [NF]

 then using the very long url from my prior post, i was able to reproduce the error, and that fact that you have fixed it in R2; however, the long URL is giving your code other problems.  From the log file:
Fri Oct 24 13:27:51 2008 - GetServerVariable_AutoFree: getting 'url'
Fri Oct 24 13:27:51 2008 - GetServerVariable_AutoFree - no joy (GetLastError()=1413)
Fri Oct 24 13:27:51 2008 - GetServerVariable_AutoFree: 128 bytes
Fri Oct 24 13:27:51 2008 - GetServerVariable_AutoFree: result ''
Fri Oct 24 13:27:51 2008 - GetHeader_AutoFree: getting 'url'
Fri Oct 24 13:27:51 2008 - GetHeader_AutoFree failed (GetLastError()=122)
Fri Oct 24 13:27:51 2008 - GetHeader_AutoFree: 2135 bytes
Fri Oct 24 13:27:51 2008 - GetHeader_AutoFree: result ' ø'

the result return by GetHeader_AutoFree (garbage) is then used later.
I hope I have giving you enough to reproduce the error in 1.2.15R@ and to test for similar problems in version 2+.
Fred

Coordinator
Oct 24, 2008 at 7:17 PM
Fred, your original description was a good test case, and I used it.  (I didn't need the actual URL)
But I used it from IE.  You're telling me that IE cuts off the URL and FF does not - maybe that is why I could not repro the problem myself.
I don't have FF installed.  I guess I should get it.

Yes, I think I have enough info now.
I'll get back to ya.
Coordinator
Oct 25, 2008 at 1:22 AM
what platform are you running on? WS2008 WS2003 Vista XP ?
Oct 25, 2008 at 1:25 AM
XP
----- Original Message -----
From: [email removed]
To: [email removed]
Sent: Friday, October 24, 2008 8:22 PM
Subject: Re: Bug in 1.2.15 final [IIRF:38370]

From: Cheeso

what platform are you running on? WS2008 WS2003 Vista XP ?
Coordinator
Oct 25, 2008 at 1:39 AM
got it.  Right, I just reproduced this on an XP box.  It does not happen on IIS7 (Vista WS2008).
Oct 25, 2008 at 1:41 AM
sorry should have mentioned that - also happens on 2003 server (where I deploy aps), dunno bout 2008 server
----- Original Message -----
From: [email removed]
To: [email removed]
Sent: Friday, October 24, 2008 8:40 PM
Subject: Re: Bug in 1.2.15 final [IIRF:38370]

From: Cheeso

got it. Right, I just reproduced this on an XP box. It does not happen on IIS7 (Vista WS2008).
Coordinator
Oct 25, 2008 at 2:11 AM
Fred, the bad pointer is being returned when the length of the incoming URL exceeds the maximum length of URL allowed in IIS (2048). 
This never happens with IE, but obviously can happen with FF or some other http client. 
In this case IIRF will just fail fast and return a 404. 
IIRF cannot depend on IIS correctly processing  a URL that exceeds the designated max length, so there is no sense trying.  

That work for you?

This change is in v1.2.15 R3 (available now).
Oct 25, 2008 at 2:30 AM
Any fix is ok by me. I think a 400 - bad request might be more accurate, or 413 Entity Too Large (like apache). Its your call.
Thanks for fixing the problem.
Fred
----- Original Message -----
From: [email removed]
To: [email removed]
Sent: Friday, October 24, 2008 9:11 PM
Subject: Re: Bug in 1.2.15 final [IIRF:38370]

From: Cheeso

Fred, the bad pointer is being returned when the length of the incoming URL exceeds the maximum length of URL allowed in IIS (2048).
This never happens with IE, but obviously can happen with FF or some other http client.
In this case IIRF will just fail fast and return a 404.
IIRF cannot depend on IIS correctly processing a URL that exceeds the designated max length, so there is no sense trying.

That work for you?

This change is in v1.2.15 R3 (available now).
Coordinator
Oct 25, 2008 at 3:28 AM
Hmmm, I like 413 better.

From: [email removed]
Sent: Friday, October 24, 2008 7:30 PM
To: [email removed]
Subject: Re: Bug in 1.2.15 final [IIRF:38370]

From: cfneedham

Any fix is ok by me. I think a 400 - bad request might be more accurate, or 413 Entity Too Large (like apache). Its your call.
Thanks for fixing the problem.
Fred
----- Original Message -----
From: [email removed]
To: [email removed]
Sent: Friday, October 24, 2008 9:11 PM
Subject: Re: Bug in 1.2.15 final [IIRF:38370]

From: Cheeso

Fred, the bad pointer is being returned when the length of the incoming URL exceeds the maximum length of URL allowed in IIS (2048).
This never happens with IE, but obviously can happen with FF or some other http client.
In this case IIRF will just fail fast and return a 404.
IIRF cannot depend on IIS correctly processing a URL that exceeds the designated max length, so there is no sense trying.

That work for you?

This change is in v1.2.15 R3 (available now).