Bug #9157

rb_readlink() calls rb_str_modify_expand() too early

Added by Maciek Nowacki over 1 year ago. Updated about 1 year ago.

[ruby-core:58592]
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

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 over 1 year 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 over 1 year 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 Anonymous over 1 year ago

unsubscribe

#2 Updated by Nobuyoshi Nakada over 1 year ago

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

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]

#3 Updated by Nobuyoshi Nakada over 1 year ago

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

#4 Updated by Maciek Nowacki over 1 year 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.

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]

#5 Updated by Victor Pereira over 1 year ago

does it deserves a CVE?

#6 Updated by Nobuyoshi Nakada about 1 year ago

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

#7 Updated by Tomoyuki Chikanaga about 1 year 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