You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2020/09/29 09:45:04 UTC
[GitHub] [camel] coheigea opened a new pull request #4314: CAMEL-15591 - Put a configurable limit on the size of unzipped data u…
coheigea opened a new pull request #4314:
URL: https://github.com/apache/camel/pull/4314
…sing camel-zipfile + camel-tarfile
For camel-zipfile + camel-tarfile, there is no limit on the size of uncompressed data, potentially leading to an OOM Error. Instead we should have a configurable limit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] coheigea edited a comment on pull request #4314: CAMEL-15591 - Put a configurable limit on the size of unzipped data u…
Posted by GitBox <gi...@apache.org>.
coheigea edited a comment on pull request #4314:
URL: https://github.com/apache/camel/pull/4314#issuecomment-701274914
@oscerd Done.
@bedlaj 1GB is admittedly arbitrary - I just chose it as it seems unlikely to me for most use-cases that one will be uncompressing to a size > 1GB. Agreed about having to update the migration guide if this change is accepted. I fixed the Checkstyle issue, thanks.
@davsclaus The problem is that with one of the files here (https://www.bamsoftware.com/hacks/zipbomb/) it can potentially unzip to 4.5 PB, so spooling to disk won't help. I don't really mind disabling the default limit, but then this is not "secure by default". Would a higher limit of say 10GB be more acceptable? Let me know + I'll change it to whatever you suggest.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] coheigea commented on pull request #4314: CAMEL-15591 - Put a configurable limit on the size of unzipped data u…
Posted by GitBox <gi...@apache.org>.
coheigea commented on pull request #4314:
URL: https://github.com/apache/camel/pull/4314#issuecomment-702245083
@davsclaus Thanks for the feedback - I've left the default limit at 1G and updated the migration guide as well. Let me know if it's OK to merge...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on pull request #4314: CAMEL-15591 - Put a configurable limit on the size of unzipped data u…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on pull request #4314:
URL: https://github.com/apache/camel/pull/4314#issuecomment-702359458
Yeah I think its fine.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on pull request #4314: CAMEL-15591 - Put a configurable limit on the size of unzipped data u…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on pull request #4314:
URL: https://github.com/apache/camel/pull/4314#issuecomment-701551994
Yeah I think an upper limit by default is more safe than not. So if you can easily set the value to 0 or -1 to disable it then that would be good. A limit of 1gb is reasonable. I dont want to have some free memory calculator that attempt to set a setting, then its better with a hardcoded limit especially for file transfer. Then you can say, okay we support up to X GB of files etc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] davsclaus commented on pull request #4314: CAMEL-15591 - Put a configurable limit on the size of unzipped data u…
Posted by GitBox <gi...@apache.org>.
davsclaus commented on pull request #4314:
URL: https://github.com/apache/camel/pull/4314#issuecomment-700696386
I think if you use stream caching then the output stream will spool to disk and therefore you dont have a memory limit problem with 1GB limit (output stream builder). @bedlaj is right with the new world of cloud and microservices vs the old way of big monolithic app servers.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] bedlaj commented on pull request #4314: CAMEL-15591 - Put a configurable limit on the size of unzipped data u…
Posted by GitBox <gi...@apache.org>.
bedlaj commented on pull request #4314:
URL: https://github.com/apache/camel/pull/4314#issuecomment-701332508
Thanks for mentioning zip bombs. I was not thinking about this attack vector while reviewing this PR. Still not sure, if there is any magic default value, which will be secure on all platforms. Maybe another option would be computing compress ration on the fly while decompressing and throw exception while it is going to exceed configured max compress ratio Eg. 50 or 100 ratio might be reasonable default.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] coheigea commented on pull request #4314: CAMEL-15591 - Put a configurable limit on the size of unzipped data u…
Posted by GitBox <gi...@apache.org>.
coheigea commented on pull request #4314:
URL: https://github.com/apache/camel/pull/4314#issuecomment-701274914
@oscerd Done.
@bedlaj 1GB is admittedly arbitrary - I just chose it as it seems unlikely to me for most use-cases that one will be uncompressing to a size > 1GB. Agreed about having to update the migration guide if this change is accepted. I don't see the Checkstyle issue any more - can you double-check?
@davsclaus The problem is that with one of the files here (https://www.bamsoftware.com/hacks/zipbomb/) it can potentially unzip to 4.5 PB, so spooling to disk won't help. I don't really mind disabling the default limit, but then this is not "secure by default". Would a higher limit of say 10GB be more acceptable? Let me know + I'll change it to whatever you suggest.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] coheigea merged pull request #4314: CAMEL-15591 - Put a configurable limit on the size of unzipped data u…
Posted by GitBox <gi...@apache.org>.
coheigea merged pull request #4314:
URL: https://github.com/apache/camel/pull/4314
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org