Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removes taint checking functions #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mhashizume
Copy link

Taint checking has been completely removed from Ruby as of Ruby 3.2.0. This commit removes taint checking functions from ruby- shadow.

See https://bugs.ruby-lang.org/issues/16131

Taint checking has been completely removed from Ruby as of Ruby
3.2.0. This commit removes taint checking functions from ruby-
shadow.

See https://bugs.ruby-lang.org/issues/16131
@mtasaka
Copy link

mtasaka commented Dec 1, 2022

Taintedness is deprecated since 2.7.0 and this commit ruby/ruby@ffd0820 actually shows rb_tainted_str_new2 behaves the same as rb_str_new2 .

To keep compatibility, I think the following is better.

diff --git a/extconf.rb b/extconf.rb
index d17f926..04f8f97 100644
--- a/extconf.rb
+++ b/extconf.rb
@@ -10,7 +10,7 @@ require 'rbconfig'
 $CFLAGS = case RUBY_VERSION
           when /^1\.9/; '-DRUBY19'
           when /^2\./; '-DRUBY19'
-          when /^3\./; '-DRUBY19'
+          when /^3\./; '-DRUBY19 -DRUBY30'
           else; ''
           end
 
diff --git a/pwd/shadow.c b/pwd/shadow.c
index eeb96d4..e73e0db 100644
--- a/pwd/shadow.c
+++ b/pwd/shadow.c
@@ -56,8 +56,13 @@ static VALUE convert_pw_struct( struct passwd *entry )
 {
   /* Hmm. Why custom pw_change instead of sp_lstchg? */
   return rb_struct_new(rb_sPasswdEntry,
+#if defined(RUBY30)
+         rb_str_new2(entry->pw_name), /* sp_namp */
+         rb_str_new2(entry->pw_passwd), /* sp_pwdp, encryped password */
+#else
          rb_tainted_str_new2(entry->pw_name), /* sp_namp */
          rb_tainted_str_new2(entry->pw_passwd), /* sp_pwdp, encryped password */
+#endif
          Qnil, /* sp_lstchg, date when the password was last changed (in days since Jan 1, 1970) */
          Qnil, /* sp_min, days that password must stay same */
          Qnil, /* sp_max, days until password changes. */
@@ -66,7 +71,11 @@ static VALUE convert_pw_struct( struct passwd *entry )
          INT2FIX(difftime(entry->pw_change, 0) / (24*60*60)), /* pw_change */
          INT2FIX(difftime(entry->pw_expire, 0) / (24*60*60)), /* sp_expire */
          Qnil, /* sp_flag */
+#if defined(RUBY30)
+         rb_str_new2(entry->pw_class), /* sp_loginclass, user access class */
+#else
          rb_tainted_str_new2(entry->pw_class), /* sp_loginclass, user access class */
+#endif
          NULL);
 }
 
diff --git a/shadow/shadow.c b/shadow/shadow.c
index 35a77a1..5202ce5 100644
--- a/shadow/shadow.c
+++ b/shadow/shadow.c
@@ -34,8 +34,13 @@ static VALUE rb_eFileLock;
 static VALUE convert_pw_struct( struct spwd *entry ) 
 {
   return rb_struct_new(rb_sPasswdEntry,
+#if defined(RUBY30)
+		      rb_str_new2(entry->sp_namp),
+		      rb_str_new2(entry->sp_pwdp),
+#else
 		      rb_tainted_str_new2(entry->sp_namp),
 		      rb_tainted_str_new2(entry->sp_pwdp),
+#endif
 		      INT2FIX(entry->sp_lstchg),
 		      INT2FIX(entry->sp_min),
 		      INT2FIX(entry->sp_max),

@mtasaka
Copy link

mtasaka commented Dec 1, 2022

By the way created "issue" ticket as #32 .

@@ -56,8 +56,8 @@ static VALUE convert_pw_struct( struct passwd *entry )
{
/* Hmm. Why custom pw_change instead of sp_lstchg? */
return rb_struct_new(rb_sPasswdEntry,
rb_tainted_str_new2(entry->pw_name), /* sp_namp */
rb_tainted_str_new2(entry->pw_passwd), /* sp_pwdp, encryped password */
rb_str_new2(entry->pw_name), /* sp_namp */
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhashizume Does this. need to be wrapped in a macro? Are we losing anythign for older rubies?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core ruby did something similar when switching dir.c to use untainted strings:

ruby/ruby@ffd0820#diff-13026f61c17631884dc4c6ee9128710ef8801844114eaedaf0a13db649b4b0a2

In Ruby 2.7, taint has no effect on strings, and since older rubies are EOL this should be safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants