Project

General

Profile

Bug #9157

rb_readlink() calls rb_str_modify_expand() too early

Added by Maciek Nowacki about 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
ruby -v:
ruby 2.0.0p353 (2013-11-22) [x86_64-linux]
Backport:
1.9.3: DONTNEED, 2.0.0: DONE
[ruby-core:58592]

Description

If rb_readlink() (file.c) is called on a long symlink, Ruby tends to crash. This is present in -trunk. The problem is that the rb_str_modify_expand() statement is incorrectly placed before the statement 'size*=2'. Here is a patch:

--- - 2013-11-25 22:10:59.694183795 -0700
+++ file.c 2013-11-25 22:10:03.076352889 -0700
@@ -2529,8 +2529,8 @@
|| (rv < 0 && errno == ERANGE) /* quirky behavior of GPFS */
#endif
) {
- rb_str_modify_expand(v, size);
size *= 2;
+ rb_str_modify_expand(v, size);
}
if (rv < 0) {
rb_str_resize(v, 0);

file.c.patch Magnifier (310 Bytes) Maciek Nowacki, 11/26/2013 03:12 PM


Related issues

Duplicated by Ruby trunk - Bug #9158: bug in rb_readlink() in file.c (fixed) Closed

Associated revisions

Revision 43853
Added by Nobuyoshi Nakada about 2 years ago

file.c: fix buffer overflow

  • file.c (rb_readlink): fix buffer overflow on a long symlink. since rb_str_modify_expand() expands from its length but not its capacity, need to set the length properly for each expansion. [Bug #9157]

Revision 43853
Added by Nobuyoshi Nakada about 2 years ago

file.c: fix buffer overflow

  • file.c (rb_readlink): fix buffer overflow on a long symlink. since rb_str_modify_expand() expands from its length but not its capacity, need to set the length properly for each expansion. [Bug #9157]

History

#1 Updated by Nobuyoshi Nakada about 2 years ago

  • % Done changed from 0 to 100
  • Status changed from Open to Closed

This issue was solved with changeset r43853.
Maciek, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


file.c: fix buffer overflow

  • file.c (rb_readlink): fix buffer overflow on a long symlink. since rb_str_modify_expand() expands from its length but not its capacity, need to set the length properly for each expansion. [Bug #9157]

#2 [ruby-core:58598] Updated by Nobuyoshi Nakada about 2 years ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 1.9.3: DONTNEED, 2.0.0: REQUIRED

#3 [ruby-core:58611] Updated by Maciek Nowacki about 2 years ago

nobu (Nobuyoshi Nakada) wrote:

This issue was solved with changeset r43853.

Ah, I didn't realize that rb_str_modify_expand() takes a difference as its argument, not the total buffer length. This works because the function doubles the buffer size, which is the same as adding as much buffer capacity as is already present (size before *=2). My proposed fix simply made the problem less obvious. Interesting.

#4 [ruby-core:58646] Updated by Victor Pereira about 2 years ago

does it deserves a CVE?

#5 [ruby-core:58705] Updated by Nobuyoshi Nakada about 2 years ago

No, just a usual bug which aborts by local filesystem access.

#6 [ruby-core:58794] Updated by Tomoyuki Chikanaga about 2 years ago

  • Backport changed from 1.9.3: DONTNEED, 2.0.0: REQUIRED to 1.9.3: DONTNEED, 2.0.0: DONE

r43853 was backported to ruby_2_0_0 branch at r43959.

Also available in: Atom PDF