-
Notifications
You must be signed in to change notification settings - Fork 5
add some docs regarding security #335
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the prompt discussion and clarifications!
Just a typo (double negation) I spotted:
nor should (not/it) cause
@@ -490,6 +490,18 @@ generated schema (as opposed to an ad-hoc schema infered from the graph data). | |||
The build targets JDK8, so that's the minimum version. The build itself requires JDK11+. | |||
However in any case it is highly encouraged to use a modern JVM, such as JDK20. | |||
|
|||
## What about security / untrusted flatgraph files? | |||
The main potentially security issue is the following situation: You get handed an untrusted, potentially malicious, flatgraph file, and want to handle it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main potentially security issue is the following situation: You get handed an untrusted, potentially malicious, flatgraph file, and want to handle it. | |
The main potentially security issue is probably: how can you handle an untrusted - and potentially malicious - flatgraph file? |
@@ -490,6 +490,18 @@ generated schema (as opposed to an ad-hoc schema infered from the graph data). | |||
The build targets JDK8, so that's the minimum version. The build itself requires JDK11+. | |||
However in any case it is highly encouraged to use a modern JVM, such as JDK20. | |||
|
|||
## What about security / untrusted flatgraph files? | |||
The main potentially security issue is the following situation: You get handed an untrusted, potentially malicious, flatgraph file, and want to handle it. | |||
Deserializing a `.fg` file should not pop a shell / cause privilege escalation, nor should not cause excessive filesystem activity. However, it may take an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deserializing a `.fg` file should not pop a shell / cause privilege escalation, nor should not cause excessive filesystem activity. However, it may take an | |
Deserializing a `.fg` file should not be able to open a shell or cause privilege escalation, nor should it cause excessive filesystem activity. However, it may take an |
## What about security / untrusted flatgraph files? | ||
The main potentially security issue is the following situation: You get handed an untrusted, potentially malicious, flatgraph file, and want to handle it. | ||
Deserializing a `.fg` file should not pop a shell / cause privilege escalation, nor should not cause excessive filesystem activity. However, it may take an | ||
unbounded amount of time and memory, potentially leading to an OOM crash of the JVM that might not be recoverable from within the JVM by catching some exceptions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unbounded amount of time and memory, potentially leading to an OOM crash of the JVM that might not be recoverable from within the JVM by catching some exceptions. | |
unbounded amount of time and memory, potentially leading to an OutOfMemoryError in which is typically not recoverable and will bring down the entire JVM. |
|
||
The easiest malicious but completely valid `.fg` file in that vein is a ZIP-bomb. We take care not to decompress graphs into the filesystem, but we do decompress them into memory. | ||
|
||
If you need to handle untrusted `.fg` files, then we recommend some form of sandboxing in order to limit the DoS impact. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you need to handle untrusted `.fg` files, then we recommend some form of sandboxing in order to limit the DoS impact. | |
If you need to handle untrusted `.fg` files you should really sandbox your process, in order to limit the DoS impact. |
|
||
If you need to handle untrusted `.fg` files, then we recommend some form of sandboxing in order to limit the DoS impact. | ||
|
||
If you do decide against our recommendation to write your own code to "sanity check" potentially malicious `.fg` files before attempting to deserialize them, then we'd be happy for your feedback and PRs. (also beware of potential parser differentials -- e.g. the manifest json can be reached either via the offset from the file header, or via `tail -n 1`, and these may very well be different manifests) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do decide against our recommendation to write your own code to "sanity check" potentially malicious `.fg` files before attempting to deserialize them, then we'd be happy for your feedback and PRs. (also beware of potential parser differentials -- e.g. the manifest json can be reached either via the offset from the file header, or via `tail -n 1`, and these may very well be different manifests) | |
You should always sanity check potentially malicious `.fg` files before attempting to deserialize them. If there's anything we can add in flatgraph's Deserializer, we'd be happy for your feedback and PRs. | |
N.b. also beware of potential parser differentials -- e.g. the manifest json can be reached either via the offset from the file header, or via `tail -n 1`, and these may very well be different manifests. |
This adds some clarifications regarding security goals. Big thanks to https://github.com/xavierpinho ❤️ for mentioning that here https://github.com/joernio/flatgraph/security/advisories/GHSA-jqmx-3x2p-69vh
Any improvement / rewording ideas?