ethereum / ethereumj

DEPRECATED! Java implementation of the Ethereum yellowpaper. For JSON-RPC and other client features check Ethereum Harmony
GNU Lesser General Public License v3.0
2.18k stars 1.1k forks source link

Unsafe Java object serialization #1161

Open mkalinin opened 6 years ago

mkalinin commented 6 years ago

What's wrong

Ethash class uses pure Java object serialization to store light and full datasets. As @ThingToNO pointed out, this serialization in its pure form can be exploited, additional information can be found here. However, in this particular case it doesn't look valuable for attacker.

How to fix

Looks like the easiest way is to use custom serialization.

mkalinin commented 6 years ago

Special thanks to @ThingToNO for investigation. It's much appreciated 👍

rschultheis commented 5 years ago

Hi @mkalinin 👋 I've recently noticed that CVE-2018-15890 has been published on this issue. It does not appear based on this issue that a fix has been made. Is a fix planned for this issue (or has it potentially already been fixed)?

mkalinin commented 5 years ago

@rschultheis It's not yet been fixed. We probably tack on that during Ethash to ProgPoW update.

rschultheis commented 5 years ago

Hi @mkalinin . I am evaluating whether this CVE should included in GitHub's advisory dataset, thus sending security alerts to clients. Something seems out of sync between the CVE, and this issue, in terms of how severe this vulnerability is.

In the parent comment above it says this implying this is not parituclarly exploitable:

However, in this particular case it doesn't look valuable for attacker.

In the CVE however, it has a Critical base severity as says:

When a node syncs and mines a new block, arbitrary OS commands can be run on the server.

We understand sometimes that CVEs are issues for things that are not truly exploitable vulnerabilities, but in this case it is somewhat unclear. Could you help give us some context on this issue and whether it represents a true critical vulnerability?

🙇 Thanks!