[Mono-list] Gonzalo: Another patch for BUG in vhosts

Andrew Arnott AndrewArnott@byu.edu
Mon, 24 May 2004 14:39:57 -0600


This is a multi-part message in MIME format.

------=_NextPart_000_006E_01C4419C.FB8864A0
Content-Type: multipart/mixed;
	boundary="----=_NextPart_001_006F_01C4419C.FB8864A0"


------=_NextPart_001_006F_01C4419C.FB8864A0
Content-Type: multipart/alternative;
	boundary="----=_NextPart_002_0070_01C4419C.FB8864A0"


------=_NextPart_002_0070_01C4419C.FB8864A0
Content-Type: text/plain;
	charset="US-ASCII"
Content-Transfer-Encoding: 7bit

The last patch I submitted (which is already in CVS) has a bug in it (sorry,
I'm not as good at C as I am at C#).  This patch corrects the bug.  Below is
a description of the problem:

 

The method that forks out mod-mono-server.exe has a 14 element array in
which it fills the switches that it will pass mod-mono-server.exe.  It then
calls execv, which takes the array of arguments, which array is terminated
by the first NULL array element.  So if you have an array like this:

 

--applications abc -root /home NULL NULL NULL NULL .

 

Then the arguments passed in are the first four array elements.  The bug did
this:

 

--applications abc -root /home NULL NULL -appconfigdir /somedir NULL NULL

 

Because of the NULLs that appeared in the middle, --appconfigdir would never
get passed to the forked process.  My patch corrects this by consolidating
all switches toward the front of the array, and leaves all NULLs at the end
of it.

 

I also added comments explaining this (briefly) where future bugs are most
likely to occur.

 

Andrew Arnott

 


------=_NextPart_002_0070_01C4419C.FB8864A0
Content-Type: text/html;
	charset="US-ASCII"
Content-Transfer-Encoding: quoted-printable

<html xmlns:o=3D"urn:schemas-microsoft-com:office:office" =
xmlns:w=3D"urn:schemas-microsoft-com:office:word" =
xmlns=3D"http://www.w3.org/TR/REC-html40">

<head>
<META HTTP-EQUIV=3D"Content-Type" CONTENT=3D"text/html; =
charset=3Dus-ascii">
<meta name=3DGenerator content=3D"Microsoft Word 11 (filtered medium)">
<style>
<!--
 /* Style Definitions */
 p.MsoNormal, li.MsoNormal, div.MsoNormal
	{margin:0in;
	margin-bottom:.0001pt;
	font-size:12.0pt;
	font-family:"Times New Roman";}
a:link, span.MsoHyperlink
	{color:blue;
	text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
	{color:purple;
	text-decoration:underline;}
p.Code, li.Code, div.Code
	{margin:0in;
	margin-bottom:.0001pt;
	font-size:10.0pt;
	font-family:"Courier New";}
span.EmailStyle18
	{mso-style-type:personal-compose;
	font-family:Arial;
	color:windowtext;}
@page Section1
	{size:8.5in 11.0in;
	margin:1.0in 1.25in 1.0in 1.25in;}
div.Section1
	{page:Section1;}
-->
</style>

</head>

<body lang=3DEN-US link=3Dblue vlink=3Dpurple>

<div class=3DSection1>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'>The last patch I submitted (which is already in CVS) =
has a
bug in it (sorry, I&#8217;m not as good at C as I am at C#). &nbsp;This =
patch
corrects the bug.&nbsp; Below is a description of the =
problem:<o:p></o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'><o:p>&nbsp;</o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'>The method that forks out mod-mono-server.exe has a =
14
element array in which it fills the switches that it will pass
mod-mono-server.exe. &nbsp;It then calls execv, which takes the array of =
arguments,
which array is terminated by the first NULL array element. &nbsp;So if =
you have an
array like this:<o:p></o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'><o:p>&nbsp;</o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'>--applications abc &#8211;root /home NULL NULL NULL =
NULL &#8230;<o:p></o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'><o:p>&nbsp;</o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'>Then the arguments passed in are the first four array
elements.&nbsp; The bug did this:<o:p></o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'><o:p>&nbsp;</o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'>--applications abc &#8211;root /home NULL NULL =
&#8211;appconfigdir
/somedir NULL NULL<o:p></o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'><o:p>&nbsp;</o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'>Because of the NULLs that appeared in the middle,
--appconfigdir would never get passed to the forked process. &nbsp;My =
patch corrects
this by consolidating all switches toward the front of the array, and =
leaves
all NULLs at the end of it.<o:p></o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'><o:p>&nbsp;</o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'>I also added comments explaining this (briefly) where =
future
bugs are most likely to occur.<o:p></o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'><o:p>&nbsp;</o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'>Andrew Arnott<o:p></o:p></span></font></p>

<p class=3DMsoNormal><font size=3D2 face=3DArial><span =
style=3D'font-size:10.0pt;
font-family:Arial'><o:p>&nbsp;</o:p></span></font></p>

</div>

</body>

</html>

------=_NextPart_002_0070_01C4419C.FB8864A0--

------=_NextPart_001_006F_01C4419C.FB8864A0
Content-Type: application/octet-stream;
	name="vhosts2.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="vhosts2.patch"

--- mod_mono/src/mod_mono.c.before	2004-05-24 12:51:33.000000000 -0600
+++ mod_mono/src/mod_mono.c	2004-05-24 12:54:53.000000000 -0600
@@ -110,7 +110,7 @@
 #define WAPIDIR				"/tmp"
 #define DOCUMENT_ROOT		NULL
 #define APPCONFIG_FILE		NULL
-#define APPCONFIG_DIR		NULL
+#define APPCONFIG_DIR		"/usr/local/apache/conf/webapps"
 #define SOCKET_FILE		"/tmp/mod_mono_server"
 
 /* define this to get tons of messages in the log */
@@ -634,7 +634,9 @@
 	pid_t pid;
 	int status;
 	int i;
-	char *argv [14];
+	const int maxargs = 14;
+	char *argv [maxargs];
+	int argi;
 	char *path;
 	char *tmp;
 	char *monodir;
@@ -691,36 +693,33 @@
 	mkdir (wapidir, 0700);
 	chmod (wapidir, 0700);
 	setenv ("MONO_SHARED_DIR", server_conf->wapidir, 1);
-	argv [0] = server_conf->executable_path;
-	argv [1] = server_conf->server_path;
-	argv [2] = "--filename";
-	argv [3] = server_conf->filename;
-	argv [4] = "--applications";
-	argv [5] = server_conf->applications;
-	argv [6] = "--nonstop";
+	for( argi = 0; argi < maxargs; argi++ ) argv [argi] = NULL;
+	argi = 0;
+	argv [argi++] = server_conf->executable_path;
+	argv [argi++] = server_conf->server_path;
+	argv [argi++] = "--filename";
+	argv [argi++] = server_conf->filename;
+	argv [argi++] = "--applications";
+	argv [argi++] = server_conf->applications;
+	argv [argi++] = "--nonstop";
         if (server_conf->document_root != NULL) {
-                argv [7] = "--root";
-                argv [8] = server_conf->document_root;
-        } else {
-                argv[7] = NULL;
-                argv[8] = NULL;
+                argv [argi++] = "--root";
+                argv [argi++] = server_conf->document_root;
         }
 	if (server_conf->appconfig_file != NULL) {
-		argv[9] = "--appconfigfile";
-		argv[10] = server_conf->appconfig_file;
-	} else {
-		argv[9] = NULL;
-		argv[10] = NULL;
+		argv [argi++] = "--appconfigfile";
+		argv [argi++] = server_conf->appconfig_file;
 	}
 	if (server_conf->appconfig_dir != NULL) {
-		argv[11] = "--appconfigdir";
-		argv[12] = server_conf->appconfig_dir;
-	} else {
-		argv[11] = NULL;
-		argv[12] = NULL;
+		argv [argi++] = "--appconfigdir";
+		argv [argi++] = server_conf->appconfig_dir;
 	}
+	// The last element in the argv array must always be NULL
+	// to terminate the array for execv().
 
-        argv [13] = NULL;
+	// Any new argi++'s that are added here must also increase
+	// the maxargs argument at the top of this method to prevent
+	// array out-of-bounds. 
 
 	ap_log_error (APLOG_MARK, APLOG_DEBUG,
 		      STATUS_AND_SERVER,

------=_NextPart_001_006F_01C4419C.FB8864A0--

------=_NextPart_000_006E_01C4419C.FB8864A0
Content-Type: application/x-pkcs7-signature;
	name="smime.p7s"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
	filename="smime.p7s"

MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAQAAoIIJkTCCAxkw
ggKCoAMCAQICAwxX1zANBgkqhkiG9w0BAQQFADBiMQswCQYDVQQGEwJaQTElMCMGA1UEChMcVGhh
d3RlIENvbnN1bHRpbmcgKFB0eSkgTHRkLjEsMCoGA1UEAxMjVGhhd3RlIFBlcnNvbmFsIEZyZWVt
YWlsIElzc3VpbmcgQ0EwHhcNMDQwNTE5MTYyMDMwWhcNMDUwNTE5MTYyMDMwWjCBwDEfMB0GA1UE
AxMWVGhhd3RlIEZyZWVtYWlsIE1lbWJlcjEjMCEGCSqGSIb3DQEJARYUQW5kcmV3QXJub3R0QGJ5
dS5lZHUxJDAiBgkqhkiG9w0BCQEWFUFuZHJld19Bcm5vdHRAYnl1LmVkdTEpMCcGCSqGSIb3DQEJ
ARYadGhpc2lzdGhlcGxhY2VAaG90bWFpbC5jb20xJzAlBgkqhkiG9w0BCQEWGHJlbGF0ZV93ZWJt
YXN0ZXJAYnl1LmVkdTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEArlzOF5Csqi69H7O9cETu
bfnVij+sURASLZVKRZHxPZwlaMEuOQeGVkXkWxGjw12LbkbXcR86tfuoAmR0okMtEzl0ygYMHT8o
wqvO2CRqTUMkUdk0ORutlzG1bsNpyWxwxYrpsiOdUuyvF2w8+xJSyTypJKGE3IN5aK9od8un+ucC
AwEAAaN+MHwwbAYDVR0RBGUwY4EUQW5kcmV3QXJub3R0QGJ5dS5lZHWBFUFuZHJld19Bcm5vdHRA
Ynl1LmVkdYEadGhpc2lzdGhlcGxhY2VAaG90bWFpbC5jb22BGHJlbGF0ZV93ZWJtYXN0ZXJAYnl1
LmVkdTAMBgNVHRMBAf8EAjAAMA0GCSqGSIb3DQEBBAUAA4GBAEJwfK49WD9qTJAXXOovageUPPdu
OXDeUAaBUQXNBR0/voePXxDOaiESAnlaEpiGQZgdTio9aEearkADBjMFX37bV9G5oRiBgjAU+rQj
wUweqn+4ZqyJL2xeOQ+eqGMbAg4d/BHYMnbO9FhYxQVVwbJnPeNRHhMpFRIYTDWjgVBTMIIDLTCC
ApagAwIBAgIBADANBgkqhkiG9w0BAQQFADCB0TELMAkGA1UEBhMCWkExFTATBgNVBAgTDFdlc3Rl
cm4gQ2FwZTESMBAGA1UEBxMJQ2FwZSBUb3duMRowGAYDVQQKExFUaGF3dGUgQ29uc3VsdGluZzEo
MCYGA1UECxMfQ2VydGlmaWNhdGlvbiBTZXJ2aWNlcyBEaXZpc2lvbjEkMCIGA1UEAxMbVGhhd3Rl
IFBlcnNvbmFsIEZyZWVtYWlsIENBMSswKQYJKoZIhvcNAQkBFhxwZXJzb25hbC1mcmVlbWFpbEB0
aGF3dGUuY29tMB4XDTk2MDEwMTAwMDAwMFoXDTIwMTIzMTIzNTk1OVowgdExCzAJBgNVBAYTAlpB
MRUwEwYDVQQIEwxXZXN0ZXJuIENhcGUxEjAQBgNVBAcTCUNhcGUgVG93bjEaMBgGA1UEChMRVGhh
d3RlIENvbnN1bHRpbmcxKDAmBgNVBAsTH0NlcnRpZmljYXRpb24gU2VydmljZXMgRGl2aXNpb24x
JDAiBgNVBAMTG1RoYXd0ZSBQZXJzb25hbCBGcmVlbWFpbCBDQTErMCkGCSqGSIb3DQEJARYccGVy
c29uYWwtZnJlZW1haWxAdGhhd3RlLmNvbTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA1GnX
1LCUZFtx6UfYDFG26nKRsIRefS0Nj3sS34UldSh0OkIsYyeflXtL734Zhx2G6qPduc6WZBrCFG5E
rHzmj+hND3EfQDimAKOHePb5lIZererAXnbr2RSjXW56fAylS1V/Bhkpf56aJtVquzgkCGqYx7Ha
o5iR/Xnb5VrEHLkCAwEAAaMTMBEwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQQFAAOBgQDH
7JJ+Tvj1lqVnYiqk8E0RYNBvjWBYYawmu1I1XAjPMPuoSpaKH2JCI4wXD/S6ZJwXrEcp352YXtJs
YHFcoqzceePnbgBHH7UNKOgCneSa/RP0ptl8sfjcXyMmCZGAc9AUG95DqYMl8uacLxXK/qarigd1
iwzdUYRr5PjRzneigTCCAz8wggKooAMCAQICAQ0wDQYJKoZIhvcNAQEFBQAwgdExCzAJBgNVBAYT
AlpBMRUwEwYDVQQIEwxXZXN0ZXJuIENhcGUxEjAQBgNVBAcTCUNhcGUgVG93bjEaMBgGA1UEChMR
VGhhd3RlIENvbnN1bHRpbmcxKDAmBgNVBAsTH0NlcnRpZmljYXRpb24gU2VydmljZXMgRGl2aXNp
b24xJDAiBgNVBAMTG1RoYXd0ZSBQZXJzb25hbCBGcmVlbWFpbCBDQTErMCkGCSqGSIb3DQEJARYc
cGVyc29uYWwtZnJlZW1haWxAdGhhd3RlLmNvbTAeFw0wMzA3MTcwMDAwMDBaFw0xMzA3MTYyMzU5
NTlaMGIxCzAJBgNVBAYTAlpBMSUwIwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5KSBMdGQu
MSwwKgYDVQQDEyNUaGF3dGUgUGVyc29uYWwgRnJlZW1haWwgSXNzdWluZyBDQTCBnzANBgkqhkiG
9w0BAQEFAAOBjQAwgYkCgYEAxKY8VXNV+065yplaHmjAdQRwnd/p/6Me7L3N9VvyGna9fww6YfK/
Uc4B1OVQCjDXAmNaLIkVcI7dyfArhVqqP3FWy688Cwfn8R+RNiQqE88r1fOCdz0Dviv+uxg+B79A
gAJk16emu59l0cUqVIUPSAR/p7bRPGEEQB5kGXJgt/sCAwEAAaOBlDCBkTASBgNVHRMBAf8ECDAG
AQH/AgEAMEMGA1UdHwQ8MDowOKA2oDSGMmh0dHA6Ly9jcmwudGhhd3RlLmNvbS9UaGF3dGVQZXJz
b25hbEZyZWVtYWlsQ0EuY3JsMAsGA1UdDwQEAwIBBjApBgNVHREEIjAgpB4wHDEaMBgGA1UEAxMR
UHJpdmF0ZUxhYmVsMi0xMzgwDQYJKoZIhvcNAQEFBQADgYEASIzRUIPqCy7MDaNmrGcPf6+svsIX
oUOWlJ1/TCG4+DYfqi2fNi/A9BxQIJNwPP2t4WFiw9k6GX6EsZkbAMUaC4J0niVQlGLH2ydxVyWN
3amcOY6MIE9lX5Xa9/eH1sYITq726jTlEBpbNU1341YheILcIRk13iSx0x1G/11fZU8xggLPMIIC
ywIBATBpMGIxCzAJBgNVBAYTAlpBMSUwIwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5KSBM
dGQuMSwwKgYDVQQDEyNUaGF3dGUgUGVyc29uYWwgRnJlZW1haWwgSXNzdWluZyBDQQIDDFfXMAkG
BSsOAwIaBQCgggG8MBgGCSqGSIb3DQEJAzELBgkqhkiG9w0BBwEwHAYJKoZIhvcNAQkFMQ8XDTA0
MDUyNDIwMzk1NlowIwYJKoZIhvcNAQkEMRYEFGXqGHtu4ZlSon8Uz8ElVQALLK36MGcGCSqGSIb3
DQEJDzFaMFgwCgYIKoZIhvcNAwcwDgYIKoZIhvcNAwICAgCAMA0GCCqGSIb3DQMCAgFAMAcGBSsO
AwIHMA0GCCqGSIb3DQMCAgEoMAcGBSsOAwIaMAoGCCqGSIb3DQIFMHgGCSsGAQQBgjcQBDFrMGkw
YjELMAkGA1UEBhMCWkExJTAjBgNVBAoTHFRoYXd0ZSBDb25zdWx0aW5nIChQdHkpIEx0ZC4xLDAq
BgNVBAMTI1RoYXd0ZSBQZXJzb25hbCBGcmVlbWFpbCBJc3N1aW5nIENBAgMMV9cwegYLKoZIhvcN
AQkQAgsxa6BpMGIxCzAJBgNVBAYTAlpBMSUwIwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5
KSBMdGQuMSwwKgYDVQQDEyNUaGF3dGUgUGVyc29uYWwgRnJlZW1haWwgSXNzdWluZyBDQQIDDFfX
MA0GCSqGSIb3DQEBAQUABIGAZjGnLHJICUm4OxwX6DsEjAsshU4hrmTknihiojzEVlZMEY23dWEM
ljtxuXU29hJO2z4DbWVanjdZNXgJSQHAq+16isTPiPknXI7lCdF/GFhIk/TNGrhyQKmeVuGkIwE3
2Uft/Wqidhzt67pG+c1ZlWMldQhEBtW6A0GgBM8nF6MAAAAAAAA=

------=_NextPart_000_006E_01C4419C.FB8864A0--