Bug #9157

rb_readlink() calls rb_str_modify_expand() too early

Added by Maciek Nowacki almost 2 years ago. Updated over 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 almost 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 almost 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 Anonymous almost 2 years ago

unsubscribe

#2 Updated by Nobuyoshi Nakada almost 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]

#3 Updated by Nobuyoshi Nakada almost 2 years 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 almost 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.

#5 Updated by Victor Pereira almost 2 years ago

does it deserves a CVE?

#6 Updated by Nobuyoshi Nakada almost 2 years ago

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

#7 Updated by Tomoyuki Chikanaga over 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