[ruby-core:18375] Stack "Overflow" Patch

< ^ > P N |<>| ^ _>< --- | ~ ...Help
--Apple-Mail-26-694330091
Content-Type: multipart/mixed;
	boundary=Apple-Mail-25-694329961


--Apple-Mail-25-694329961
Content-Type: text/plain;
	charset=WINDOWS-1252;
	format=flowed;
	delsp=yes
Content-Transfer-Encoding: quoted-printable

In my spare time, I've been working on a OSX InputManager that =20
automatically injects a ruby console into every application (inspired =20=

by FScript). I noticed several crashes in ruby code, and I think one =20
of them might be a bug in ruby itself. There are a couple more in =20
rubycocoa, but that's a subject for another list. I should also =20
mention that I am not a ruby guru, so my understanding here may be =20
flawed. Please correct anything I get wrong :)

Ruby apparently integrates its own stack with the C stack, so it looks =20=

for overflows by measuring the C stack at certain points and making =20
sure it never gets too large. Unfortunately, when writing code that re-=20=

enters ruby, this mechanism can become confused. For instance, if I =20
have a 10k stack when I first call into ruby, rb_gc_stack_start is set =20=

to C_STACK_START - 10k if my stack grows down. Ruby's stack usage is =20
computed (in pseudocode again) by STACK_END - rb_gc_stack_start. If I =20=

then re-enter from a 1k stack in C, however, STACK_END - =20
rb_gc_stack_start becomes negative. Since pointers aren't signed, the =20=

stack size looks huge and ruby reports a stack overflow.

The least intrusive solution I could think of was to effectively take =20=

the absolute value of the stack length result. It isn't elegant =96 with =
=20
a hugely varying pre-entry stack, the same problem could occur =96 but =20=

it solves my immediate problem without adding any code to what seems =20
like a security sensitive part of ruby. This patch is =20
stack_check_1.patch, attached below.

--Apple-Mail-25-694329961
Content-Disposition: attachment;
	filename=stack_check_1.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="stack_check_1.patch"
Content-Transfer-Encoding: 7bit

Index: gc.c
===================================================================
--- gc.c	(revision 18781)
+++ gc.c	(working copy)
@@ -515,14 +515,8 @@
 # endif
 # define STACK_END (stack_end)
 #endif
-#if STACK_GROW_DIRECTION < 0
-# define STACK_LENGTH  (rb_gc_stack_start - STACK_END)
-#elif STACK_GROW_DIRECTION > 0
-# define STACK_LENGTH  (STACK_END - rb_gc_stack_start + 1)
-#else
 # define STACK_LENGTH  ((STACK_END < rb_gc_stack_start) ? rb_gc_stack_start - STACK_END\
                                            : STACK_END - rb_gc_stack_start + 1)
-#endif
 #if STACK_GROW_DIRECTION > 0
 # define STACK_UPPER(x, a, b) a
 #elif STACK_GROW_DIRECTION < 0

--Apple-Mail-25-694329961
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed;
	delsp=yes
Content-Transfer-Encoding: 7bit




A more proper solution would be to cut off the stack length  
computation at 0. It seems risky, and I'd love to hear from the more  
security-minded denizens of this list, but I can't really think of a  
situation where a "negative" stack length could be exploited. This is  
stack_check_2.patch.

--Apple-Mail-25-694329961
Content-Disposition: attachment;
	filename=stack_check_2.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="stack_check_2.patch"
Content-Transfer-Encoding: 7bit

Index: gc.c
===================================================================
--- gc.c	(revision 18781)
+++ gc.c	(working copy)
@@ -516,9 +516,9 @@
 # define STACK_END (stack_end)
 #endif
 #if STACK_GROW_DIRECTION < 0
-# define STACK_LENGTH  (rb_gc_stack_start - STACK_END)
+# define STACK_LENGTH  ((STACK_END > rb_gc_stack_start) ? 0 : (rb_gc_stack_start - STACK_END))
 #elif STACK_GROW_DIRECTION > 0
-# define STACK_LENGTH  (STACK_END - rb_gc_stack_start + 1)
+# define STACK_LENGTH  ((rb_gc_stack_start > STACK_END) ? 0 : (STACK_END - rb_gc_stack_start + 1))
 #else
 # define STACK_LENGTH  ((STACK_END < rb_gc_stack_start) ? rb_gc_stack_start - STACK_END\
                                            : STACK_END - rb_gc_stack_start + 1)

--Apple-Mail-25-694329961
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed;
	delsp=yes
Content-Transfer-Encoding: 7bit





Although it isn't really a bug in ruby, I wanted to include a third  
patch for IRB that adds an option in its @CONF to turn off the  
automatic SIGINT trapping it does. It isn't too difficult to work  
around, but it seems More Proper to have an actual configuration  
option for it.

--Apple-Mail-25-694329961
Content-Disposition: attachment;
	filename=irb_notrap.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="irb_notrap.patch"
Content-Transfer-Encoding: 7bit

Index: lib/irb/ext/multi-irb.rb
===================================================================
--- lib/irb/ext/multi-irb.rb	(revision 18781)
+++ lib/irb/ext/multi-irb.rb	(working copy)
@@ -233,9 +233,10 @@
     end
   end
 
-  trap("SIGINT") do
-    @JobManager.current_job.signal_handle
-    Thread.stop
+  if !@CONF[:NOTRAP] do
+    trap("SIGINT") do
+      @JobManager.current_job.signal_handle
+      Thread.stop
+    end
   end
-
 end
Index: lib/irb.rb
===================================================================
--- lib/irb.rb	(revision 18781)
+++ lib/irb.rb	(working copy)
@@ -61,9 +61,11 @@
 
     @CONF[:IRB_RC].call(irb.context) if @CONF[:IRB_RC]
     @CONF[:MAIN_CONTEXT] = irb.context
-
-    trap("SIGINT") do
-      irb.signal_handle
+    
+    if !@CONF[:NOTRAP] do
+      trap("SIGINT") do
+        irb.signal_handle
+      end
     end
     
     catch(:IRB_EXIT) do

--Apple-Mail-25-694329961
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed;
	delsp=yes
Content-Transfer-Encoding: 7bit




All patches are against the 1.8 branch, revision 18781.


There is another problem when calling into ruby from multiple threads  
(bad, I know, but sometimes unavoidable, and it really ought to work  
within a mutex): each thread has its own stack, which predictably  
confuses the stack length computation. Before I make a patch that uses  
pthread thread-specific variables or stores ruby's stack in the heap,  
what would its chances be of actually being used (and which strategy  
would be preferable)?


Thanks,
Jonathan



--Apple-Mail-25-694329961--

--Apple-Mail-26-694330091
Content-Disposition: attachment;
	filename=smime.p7s
Content-Type: application/pkcs7-signature;
	name=smime.p7s
Content-Transfer-Encoding: base64

MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAQAAoIIDSTCCA0Uw
ggItoAMCAQICAQEwCwYJKoZIhvcNAQEFMGQxMDAuBgNVBAMMJ0pvbmF0aGFuIGRlV2VyZCdzIENl
cnRpZmljYXRlIEF1dGhvcml0eTELMAkGA1UEBhMCVVMxIzAhBgkqhkiG9w0BCQEWFGFkbWluQGpq
b28ubm8taXAuY29tMB4XDTA3MTEwNjAzNDExMVoXDTA4MTEwNTAzNDExMVowajEYMBYGA1UEAwwP
Sm9uYXRoYW4gZGVXZXJkMQswCQYDVQQIDAJDTzELMAkGA1UEBhMCVVMxDzANBgNVBAcMBkRlbnZl
cjEjMCEGCSqGSIb3DQEJARYUampvb25hdGhhbkBnbWFpbC5jb20wggEiMA0GCSqGSIb3DQEBAQUA
A4IBDwAwggEKAoIBAQChjft0hfaTuT8P0p81UUYsu2g/N9bKL0oQKHWBa6NYm6jae5N4c9HydPIA
52nP9QFMfxfj74AT4lI6VW+daO98nwg0iWo6EKECOaKXAWAeNhSUkdfZRwGudOH1Jz1NNLJour75
VnHRA1SnBqDxdln9yKdodFjvw6+wp8cS9CWcElAJwolkO07/gMcseWvL4bxigi6yy3K+0iUKn+gU
LLDB9eawp2B9hB8qgxLeUkL5q+ygLMEZS6eNKD8yf6TsnVPOu+Fz/yGctTNzrsVgPsO46XF4mnpJ
T8ekIJTwW6FnpNFHI/wlwQDncbvegfMfwaySqdApQGwY28Jr+8a2ArwJAgMBAAEwDQYJKoZIhvcN
AQEFBQADggEBABz8ktLk1Kv6cAJEdSAVtIs+85xneZVKFvSMkCgSJtvPg3GUF8pMzNzJC+Y1NPD0
G8Vh9D7RsUvvjXOZMAMpY27SM9Aoj0aU90Ib4F2tqCJMgvfIStgZVzp953mEs2jYny50ZlDncM60
LcC2E5ipQJAiMqUC5n0NJltNCpiK31iJxjXBRvjUThDneadbVNtB+pJNvn4BAniYegpEEvPp8/T8
elVwMo+siaONtv6xlk5qiTxyljShwMsXXsEyCMwEbaLkj3ThyhcsB19AqujDW+YxXea2KAertig/
Ec5kFyXp3fpz4d6p+YOAHtiWTgZ167a1p769TyP+4Z5LxgXd4YgxggLnMIIC4wIBATBpMGQxMDAu
BgNVBAMMJ0pvbmF0aGFuIGRlV2VyZCdzIENlcnRpZmljYXRlIEF1dGhvcml0eTELMAkGA1UEBhMC
VVMxIzAhBgkqhkiG9w0BCQEWFGFkbWluQGpqb28ubm8taXAuY29tAgEBMAkGBSsOAwIaBQCgggFT
MBgGCSqGSIb3DQEJAzELBgkqhkiG9w0BBwEwHAYJKoZIhvcNAQkFMQ8XDTA4MDgyMzE5MjgxOFow
IwYJKoZIhvcNAQkEMRYEFIxfluO1c4OFLNKJ8GBE6k+MBDSQMHgGCSsGAQQBgjcQBDFrMGkwZDEw
MC4GA1UEAwwnSm9uYXRoYW4gZGVXZXJkJ3MgQ2VydGlmaWNhdGUgQXV0aG9yaXR5MQswCQYDVQQG
EwJVUzEjMCEGCSqGSIb3DQEJARYUYWRtaW5Aampvby5uby1pcC5jb20CAQEwegYLKoZIhvcNAQkQ
Agsxa6BpMGQxMDAuBgNVBAMMJ0pvbmF0aGFuIGRlV2VyZCdzIENlcnRpZmljYXRlIEF1dGhvcml0
eTELMAkGA1UEBhMCVVMxIzAhBgkqhkiG9w0BCQEWFGFkbWluQGpqb28ubm8taXAuY29tAgEBMA0G
CSqGSIb3DQEBAQUABIIBAIqelZ6a2qrD7gh5V6pxVdv9cUXjPHxwvGQn+DII6zwDPgNMO75ntQlr
S/c9f6rODO46h6KeUrVl5nItaFKba5mTdh/+eHcYByDhMkjxrbJhh1XhINvBFmIlYcPlaJp7k9Hc
2jfBQsi7BoI8OlEQ/RXRD9vMa1oX6DpULFS4jraxVDKoh7KRaRcSYiGJL2oQ0jVacxm6SJmh8REd
BUlm0Fj68SIya6OiY2VPrJqaImhOa7UX2a+a0Ym2PssvGVoVjKeQgy0HhNSJVebYU/u0jxpKZ+Uc
RHvcFGXIKGdSgF6+9tx3fJWTMn4ov9Boi6KU8RP8qnNMlOj/q+G+QbRsQ5QAAAAAAAA=

--Apple-Mail-26-694330091--