From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS4713 221.184.0.0/13 X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from neon.ruby-lang.org (neon.ruby-lang.org [221.186.184.75]) by dcvr.yhbt.net (Postfix) with ESMTP id D75071F453 for ; Sun, 28 Apr 2019 04:06:54 +0000 (UTC) Received: from neon.ruby-lang.org (localhost [IPv6:::1]) by neon.ruby-lang.org (Postfix) with ESMTP id 972EF120AF9; Sun, 28 Apr 2019 13:06:49 +0900 (JST) Received: from o1678916x28.outbound-mail.sendgrid.net (o1678916x28.outbound-mail.sendgrid.net [167.89.16.28]) by neon.ruby-lang.org (Postfix) with ESMTPS id B6AC7120AF9 for ; Sun, 28 Apr 2019 13:06:47 +0900 (JST) Received: by filter0173p3mdw1.sendgrid.net with SMTP id filter0173p3mdw1-4848-5CC526D8-7 2019-04-28 04:06:48.127695652 +0000 UTC m=+200628.840351896 Received: from herokuapp.com (unknown [54.91.45.152]) by ismtpd0066p1mdw1.sendgrid.net (SG) with ESMTP id BtLr0ucCQg2J59ol5in1Ww for ; Sun, 28 Apr 2019 04:06:48.155 +0000 (UTC) Date: Sun, 28 Apr 2019 04:06:48 +0000 (UTC) From: merch-redmine@jeremyevans.net Message-ID: References: Mime-Version: 1.0 X-Redmine-MailingListIntegration-Message-Ids: 67940 X-Redmine-Project: ruby-trunk X-Redmine-Issue-Id: 15797 X-Redmine-Issue-Author: jeremyevans0 X-Redmine-Sender: jeremyevans0 X-Mailer: Redmine X-Redmine-Host: bugs.ruby-lang.org X-Redmine-Site: Ruby Issue Tracking System X-Auto-Response-Suppress: All Auto-Submitted: auto-generated X-SG-EID: =?us-ascii?Q?RVE3t853K5scBhbmJHUzZTFFeVC=2FZSUmHZ0Dc+26wcEi2CTgsF1oz0wTSSxGGN?= =?us-ascii?Q?BICDUSi2RaJI6UgVzfnKv1klTLalso+tGbX7LiV?= =?us-ascii?Q?j+2GbckpgLYKgLs27OZK64jzZIKtwJ3iyOvKjm7?= =?us-ascii?Q?SLil1Me2mQ9FV7YqbFfY=2FSlQ6jsPI1v0JVWO5fn?= =?us-ascii?Q?yAFs9fMdHUAV=2FA4VK1br4d8anpnR9DcG1QQ=3D=3D?= To: ruby-core@ruby-lang.org X-ML-Name: ruby-core X-Mail-Count: 92457 Subject: [ruby-core:92457] [Ruby trunk Feature#15797] Use realpath(3) instead of custom realpath implementation if available X-BeenThere: ruby-core@ruby-lang.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: Ruby developers List-Id: Ruby developers List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ruby-core-bounces@ruby-lang.org Sender: "ruby-core" Issue #15797 has been updated by jeremyevans0 (Jeremy Evans). File use-native-realpath-v3.patch added akr (Akira Tanaka) wrote: > PATH_MAX is dangerous. > > Quotes from http://man7.org/linux/man-pages/man3/realpath.3.html I'm not sure if this is still a problem on modern Linux, but here's an updated patch that passes a NULL pointer as the second argument to realpath(3), and then frees the pointer after the Ruby string is created. Also, just in case Windows starts defining realpath(3) in the future, make sure not to use this implementation on Windows, as the check for `path` being absolute does not work with drive letters. ---------------------------------------- Feature #15797: Use realpath(3) instead of custom realpath implementation if available https://bugs.ruby-lang.org/issues/15797#change-77807 * Author: jeremyevans0 (Jeremy Evans) * Status: Open * Priority: Normal * Assignee: * Target version: ---------------------------------------- One reason to do this is simplicity, as this approach is ~30 lines of code instead of ~200. Performance wise, this performs 25%-115% better, using the following benchmark on OpenBSD 6.5: ```ruby require 'benchmark' f = File pwd = Dir.pwd Dir.mkdir('b') unless f.directory?('b') f.write('b/a', '') unless f.file?('b/a') args = [ ["b/a", nil], ["#{pwd}/b/a", nil], ['a', 'b'], ["#{pwd}/b/a", 'b'], ["b/a", pwd] ] args.each do |path, base| print "File.realpath(#{path.inspect}, #{base.inspect}): ".ljust(50) puts Benchmark.measure{100000.times{f.realpath(path, base)}} end ``` Before: ``` File.realpath("b/a", nil): 4.330000 2.990000 7.320000 ( 7.316244) File.realpath("/home/testr/ruby/b/a", nil): 3.560000 2.680000 6.240000 ( 6.240951) File.realpath("a", "b"): 4.370000 3.080000 7.450000 ( 7.452511) File.realpath("/home/testr/ruby/b/a", "b"): 3.730000 2.640000 6.370000 ( 6.371979) File.realpath("b/a", "/home/testr/ruby"): 3.590000 2.630000 6.220000 ( 6.226824) ``` After: ``` File.realpath("b/a", nil): 1.370000 2.030000 3.400000 ( 3.400775) File.realpath("/home/testr/ruby/b/a", nil): 1.260000 2.770000 4.030000 ( 4.024957) File.realpath("a", "b"): 2.090000 1.990000 4.080000 ( 4.080284) File.realpath("/home/testr/ruby/b/a", "b"): 1.400000 2.620000 4.020000 ( 4.015505) File.realpath("b/a", "/home/testr/ruby"): 2.150000 2.760000 4.910000 ( 4.910634) ``` If someone could benchmark before/after with this patch on Linux and/or MacOS X, and post the results here, I would appreciate it. My personal reason for wanting this is that the custom realpath implementation does not work with OpenBSD's unveil(2) system call, which limits access to the file system, allowing for security similar to chroot(2), without most of the downsides. This change passes all tests except for one assertion related to taintedness. Previously, if either argument to `File.realpath` is an absolute path, then the returned value is considered not tainted. However, I believe that behavior to be incorrect, because if there is a symlink anywhere in the path, the returned value can contain a section that was taken from the file system (unreliable source) that was not marked as untainted. Example: ```ruby Dir.mkdir('b') unless File.directory?('b') File.write('b/a', '') unless File.file?('b/a') File.symlink('b', 'c') unless File.symlink?('c') path = File.realpath('c/a'.untaint, Dir.pwd.untaint) path # "/home/testr/ruby/b/a" path.tainted? # should be true, as 'b' comes from file system ``` I believe it is safer to always mark the output of realpath as tainted to prevent this issue, which is what this commit does. ---Files-------------------------------- use-native-realpath.patch (6.31 KB) use-native-realpath-v2.patch (4.64 KB) use-native-realpath-v3.patch (5.18 KB) -- https://bugs.ruby-lang.org/