New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A integer overflow in the big array serialization module leads to arbitrary code execution #7765
Comments
Comment author: @alainfrisch I think it is a well-known fact that a program should not use the generic demarshaller on untrusted data. This should perhaps be made a bit more explicit, but the document of Marshal already says: Anything can happen at run-time if the object in the file does not belong to the given type.
Are you aware of actual uses of Marshal for such operations between agents that don't trust each other? |
Comment author: maxdos Well sending corrupt data over the network should not lead to a memory corruption of my bigarray buffer ? Yes the one i created in the example, and of course this error could allow any ocaml implementation based on this library cause to be exploitable through it |
Comment author: maxdos I have to point out that you guys did a great job in all the other cases Line 108 in ea60609
Where it is implemented safe |
Comment author: @alainfrisch I agree it is worth protecting against this specific source of overflow. But let me insist: Marshal is known to not be safe, and should really not be used for communication if you don't trust the other end.
How do you draw such a conclusion? If you read an integer from an unstrusted source and call Obj.magic on it to cast it to a bytes values, you can obviously let the "attacker" read/write arbitrary memory. It only means that you should not do that. And as far as I know, nobody demarshals data received from untrusted counterparties. |
Comment author: maxdos
Yes that is my main point, you guys did great work in protecting the rest of it so why not fix this with the protection you already implemented and make ocamls stdlib more secure :) Also i agree that serializing over network is always dangerous But imagine scenarios of man-in-the-middle where a package you are serializing can be corrupted or maybe you unserialize a corrupted file you got through and overall its a small patch which adds great security to your already pretty secure marshaling interface :) |
Comment author: @alainfrisch Even when this will be fixed, demarshaling will not really become more safe against corruption or attacks. If you expect to unmarshal a value of type "bytes" but the attacker sends a marshaled integer, you will easily end up writing at arbitrary memory location controlled by the attacker. |
Comment author: @xavierleroy Well spotted, thanks! We'll fix this overflow issue by the next release, of course. @Frisch is correct that there are other ways to abuse unmarshaling at the level of the OCaml application. But those can often be avoided by post-unmarshaling data validation, for example like in Gregoire Henry's PhD work. In contrast, unsafety in the run-time system is fatal and cannot be worked around. So we'll fix the unsafety ASAP. |
Comment author: Richard Jones Just reviewing this issue for Fedora & RHEL. It looks like there isn't yet an upstream patch, is that right? |
Comment author: @alainfrisch
Which safety properties do we expect from the demarshaler, applied to a block crafted by the attacker? I don't see any protection against arbitrary memory reads in the demarshaler. For instance, CODE_STRING64 will get an arbitrary 64-bit integer length, and happily read (with readblock) that much memory from the input buffer without bound checks. If arbitrary memory reads are not bad enough, the same code also do:
or things like:
which could write at an attacker-controlled memory location (nothing prevents intern_dest from overflowing the allocated target buffer). Also, the attacker could trivially force the whole program to stop (just emit CODE_SHARED8 followed by \0). |
Comment author: maxdos @Frisch: but there we are talking about custom object correct ? |
Comment author: @alainfrisch CODE_STRING64 is not related to custom objects. The problem you reported (with bigarrays) is indeed, but I think this is irrelevant, since support for custom objects cannot be disabled. |
Comment author: @xavierleroy Proposed fix: #1718 |
Comment author: @xavierleroy Fix reviewed and merged, will be in release 4.07.0. @Frisch is right: other sanity checks are missing in the unmarshaler and should be investigated soon. |
Comment author: @alainfrisch
I'm not against adding more check, but this will add quite a bit of weight to the code (I expect the impact on performance to remain small, though). But I'm dubious at the idea of relying on post hoc checks of the unmarshal data to ensure security. I can see some use of such techniques to reduce the risk of versioning or corruption problems, but advertising this as a secure approach seems dangerous to me. Validation of unmarshaled data requires a runtime representation of the data structure definition, and involve some non negligible runtime cost. At this point, why would one want to rely on the generic marshaling rather than a type-driven serialization (which could be more compact, btw)? The difficult part would be to deal with sharing data, but this is also quite difficult for the validator, as it needs to reject sharing of mutable data with different types. I'd feel much more comfortable if we advised to "never unmarshal untrusted data" (and similarly, "never dynlink untrusted code", if this is not obvious). |
Comment author: maxdos I understand your argument regarding runtime Your suggestions sounds like: Why dont we say ocaml is not safe to use over network and then we are done ? |
Comment author: @alainfrisch
Absolutely not, and that's because you were somehow lead to believe that that I believe it's important to be clear in the documentation. Marshal should not used in contexts where an attacker can control the data. I don't believe it is, at least in any project I'm aware of, and if it were, it's unlikely that those project perform enough check on the result of Marshal to make the use safe anyway.
This has nothing to do with the use of ocaml over the network, it's about using a library documented to be unsafe on untrusted data. Of course, it's a bad idea, but this is not breaking news. |
Comment author: @alainfrisch Again: are you aware of any use of Marshal to pass data over the network, in a context where you don't trust the other side ? |
Malicious or corrupted marshaled data can result in a bigarray with impossibly large dimensions that cause overflow when computing the in-memory size of the bigarray. Disaster ensues when the data is read in a too small memory area. This commit checks for overflows when computing the in-memory size of the bigarray. This patch is based on one by Xavier Leroy and has been modified to use caml_ba_multov instead of caml_umul_overflow which is unavailable in OCaml 4.05.0. Origin: ocaml#1718 Bug: ocaml#7765 Bug-Debian: https://bugs.debian.org/895472 Bug-CVE: CVE-2018-9838
Malicious or corrupted marshaled data can result in a bigarray with impossibly large dimensions that cause overflow when computing the in-memory size of the bigarray. Disaster ensues when the data is read in a too small memory area. This commit checks for overflows when computing the in-memory size of the bigarray. This patch is based on one by Xavier Leroy and has been modified to use caml_ba_multov instead of caml_umul_overflow which is unavailable in OCaml 4.05.0. The original commit hash is 85162ee. Origin: ocaml#1718 Bug: ocaml#7765 Bug-Debian: https://bugs.debian.org/895472 Bug-CVE: CVE-2018-9838
Malicious or corrupted marshaled data can result in a bigarray with impossibly large dimensions that cause overflow when computing the in-memory size of the bigarray. Disaster ensues when the data is read in a too small memory area. This commit checks for overflows when computing the in-memory size of the bigarray. This patch is based on one by Xavier Leroy and has been modified to use caml_ba_multov instead of caml_umul_overflow which is unavailable in OCaml 4.05.0. The original commit hash is 85162ee. Origin: ocaml#1718 Bug: ocaml#7765 Bug-Debian: https://bugs.debian.org/895472 Bug-CVE: CVE-2018-9838
Malicious or corrupted marshaled data can result in a bigarray with impossibly large dimensions that cause overflow when computing the in-memory size of the bigarray. Disaster ensues when the data is read in a too small memory area. This commit checks for overflows when computing the in-memory size of the bigarray. This patch is based on one by Xavier Leroy and has been modified to use caml_ba_multov instead of caml_umul_overflow which is unavailable in OCaml 4.05.0. The original commit hash is 85162ee. Origin: ocaml#1718 Bug: ocaml#7765 Bug-Debian: https://bugs.debian.org/895472 Bug-CVE: CVE-2018-9838
Original bug ID: 7765
Reporter: maxdos
Status: resolved (set by @xavierleroy on 2018-04-12T07:37:40Z)
Resolution: fixed
Priority: urgent
Severity: major
Version: 4.06.0
Target version: 4.07.0+dev/beta2/rc1/rc2
Fixed in version: 4.07.0+dev/beta2/rc1/rc2
Category: standard library
Monitored by: @nojb "Richard Jones"
Bug description
The bigarray module in all recent ocaml versions is capable of reading in serialized (marshalled) objects from a external source which is often used for network operations and interprocess communication.
ocaml/byterun/bigarray.c
Line 458 in ea60609
A integer overflow vulnerability allows under certain circumstances an remote attacker to input a corrupt object and edit arbitrary addresses in the processes memory which leads over common techniques (in this take libc one gadget but any rop gadget or got table vector would work) to arbitrary memory access (!) and in the course also arbitrary code execution
Please check the writeup.pdf for more details !!
In variations of this the exploit is usable in all ocaml versions we tested no matter if compiled to binary or intepreted (!)
Steps to reproduce
Check out the attached archive and run the makefile (or use the precompile executable)
the exploit can be run by executing exploit.py (python2.7 and pwntools are required)
The last step of code execution is using a one-gadget attack technique and is specific to the glibc version in use (e.g. tested is debian glibc 2.24-11+deb9u1) but can with minimal amount of work be ported to any libc version on any patch level to our knowledge
Of course you can also use the exploit.py to write your own version of exploit
Additional information
This Vulnerability in ocaml was discovered by me (maximilian.tschirschnitz@gmx.de) and the exploit collaboratly developed with (philipp.hagenlocher@tum.de)
For illustration purposes it was packaged as security challenge to demonstrate the impact of the issue (see writeup.pdf)
The text was updated successfully, but these errors were encountered: