update 2018-12-12-CVE-2018-18649-Gitlab-RCE

_posts/2018-12-12-CVE-2018-18649-Gitlab-RCE.markdown

@@ -98,6 +98,114 @@ So, our bird can pass the inspection and tell us anything within a GitLab instan
 
 ![2018-12-12-CVE-2018-18649-Gitlab-RCE_1](/assets/2018-12-12-CVE-2018-18649-Gitlab-RCE_1.png)
 
-## TBD
-I'll leave the command execution part to my next post, where I'll introduce
-another interesting flexibility in Ruby.
+## Inheritance
+Inheritance is not a Ruby only feature, inheritance exists in almost every OOP
+language. It is a very helpful for designing softwares. However it also brings
+some security risks.
+
+In Ruby, there's a function `Kernel#open`
+
+``` text
+open(path [, mode [, perm]] [, opt]) → io or nil
+...
+If path starts with a pipe character ("|"), a subprocess is created, connected to the caller by a pair of pipes. The returned IO object may be used to write to the standard input and read from the standard output of this subprocess.
+```
+Which means, this function could be used to spawn sub-processes as long as the
+first char of `path` is the pipe symbol.
+
+``` text
+irb(main):001:0> Kernel.open('|id').read
+=> "uid=0(root) gid=0(root) groups=0(root)\n"
+irb(main):002:0> open('|id').read
+=> "uid=0(root) gid=0(root) groups=0(root)\n"
+```
+
+And, there are also some other functions having similar behavior, `IO::read`
+for example:
+
+``` text
+read(name, [length [, offset]] [, opt] ) → string
+Opens the file, optionally seeks to the given offset, then returns length bytes (defaulting to the rest of the file). read ensures the file is closed before returning.
+
+If name starts with a pipe character ("|"), a subprocess is created in the same way as Kernel#open, and its output is returned.
+```
+
+Which means:
+
+``` text
+irb(main):003:0> path = '|id'
+=> "|id"
+irb(main):004:0> IO.read path
+=> "uid=0(root) gid=0(root) groups=0(root)\n"
+```
+
+Well, so what's the business with inheritance here? A very very important point
+of inheritance is that, the public methods of a parent class is inherited by a
+child class, we can directly use those inherited methods in child classes as long
+as we didn't override them. In Ruby, `File` is a child class of `IO`
+
+``` text
+irb(main):005:0> File.ancestors
+=> [File, IO, File::Constants, Enumerable, Object, Kernel, BasicObject]
+```
+
+So,
+
+``` text
+irb(main):006:0> path = '|id'
+=> "|id"
+irb(main):007:0> File.read path
+(irb):7: warning: IO.read called on File to invoke external command
+=> "uid=0(root) gid=0(root) groups=0(root)\n"
+```
+
+easy huh? Till now, the arbitrary file read bug can be directly used to execute
+system commands remotely.
+
+![2018-12-12-CVE-2018-18649-Gitlab-RCE_2](/assets/2018-12-12-CVE-2018-18649-Gitlab-RCE_2.png)
+
+# Some thoughts
+
+I was quite shocked when I first discovered this `File.read` surprise. No matter
+how to explain, I still can't accept the fact that calling `File.read` method on
+a `|` started string spawns a sub-process instead of opening a file with that
+name.
+
+The fact is, not only method `read`:
+
+``` text
+pry(main)> (File.methods - File.methods(false)) & IO.methods(false)
+=> [:console,
+ :read,
+ :sysopen,
+ :for_fd,
+ :popen,
+ :foreach,
+ :binread,
+ :new,
+ :binwrite,
+ :write,
+ :copy_stream,
+ :select,
+ :pipe,
+ :open,
+ :try_convert,
+ :readlines]
+```
+
+All of these functions are inherited by `File` directly from `IO`, and some of
+them have similar behavior with `File.read`.
+
+According to the warning message printed in irb, I believe that Ruby-lang
+developers already know this and are trying to rise notice of the users. It's a
+good signal but I think it might be better to totally avoid this kind of
+unexpected behavior from the language implementation layer, such as override
+them in `File`.
+
+# Timeline
+[GitLab's bug bounty program](https://hackerone.com/gitlab) is very responsive
+and regularly updated to keep reporters in the loop.
+
++ Oct 24th, Reported to GitLab
++ Oct 29th, Patch released https://about.gitlab.com/2018/10/29/security-release-gitlab-11-dot-4-dot-3-released/
++ Oct 29th, $10000 bounty was rewarded for this report