# Incorrect security remedy for adm\_zip

**URL:** <https://community.sonarsource.com/t/incorrect-security-remedy-for-adm-zip/97929>\
**Category:** Report False-positive / False-negative...\
**Tags:** javascript, sonarqube\
**Created:** [August 23, 2023, 1:04pm UTC](https://community.sonarsource.com/t/incorrect-security-remedy-for-adm-zip/97929 "2023-08-23T13:04:36Z")\
**Posts on this page:** 3\
**Page:** 1

<div class="post-metadata">

**Author:** ![thorsent](https://avatars.discourse-cdn.com/v4/letter/t/54ee81/32.png) [@thorsent](https://community.sonarsource.com/u/thorsent)\
**Post date:** [August 23, 2023, 1:04pm UTC](https://community.sonarsource.com/t/incorrect-security-remedy-for-adm-zip/97929/1 "2023-08-23T13:04:36Z")

</div>

> **[JavaScript static code analysis: Expanding archive files without controlling...](https://rules.sonarsource.com/javascript/type/security%20hotspot/rspec-5042/)**
>
> Successful Zip Bomb attacks occur when an application expands untrusted archive files without controlling the size of the expanded data, which can
> lead to denial of service. A Zip bomb is usually a malicious archive file of a few kilobytes of...

I believe that the remedy for adm\_zip incorrectly targets high compression ratios. It’s valid and common for compression ratios to be high for highly compressible files, well over the specified THRESHOLD\_RATIO of 10 (compression _levels_ only run from 1 - 9 for the zip protocol, but ratios are dynamic).

This blocked valid and innocuous zip files for us.

```auto
let compressionRatio = entrySize / zipEntry.header.compressedSize;
    if (compressionRatio > THRESHOLD_RATIO) {
        throw 'Reached max. compression ratio';
    }

```

The file size check provides adequate protection against disk. I’m not sure if this check was meant to protect against cpu. If so, then I think a combination of file size and compression ratio would be necessary but might have to be determined heuristically.

I have not reviewed suggestions for the other zip libraries, only adm\_zip, but the same might apply.

I’m new to SonarQube, have used the lint rules for years but only recently the security rules. I’m not sure if this is the correct place to report such issues.

---

<div class="post-metadata">

**Author:** ![Hendrik\_Buchwald](https://dub1.discourse-cdn.com/sonarsource/user_avatar/community.sonarsource.com/hendrik_buchwald/32/69290_2.png) [@Hendrik\_Buchwald](https://community.sonarsource.com/u/Hendrik_Buchwald)\
**Post date:** [August 25, 2023, 11:32am UTC](https://community.sonarsource.com/t/incorrect-security-remedy-for-adm-zip/97929/4 "2023-08-25T11:32:54Z")

</div>

Hi Terry,

Welcome to the community and thanks for the report! This check is intended to prevent [zip bombs](https://en.wikipedia.org/wiki/Zip_bomb). As with the `MAX_SIZE` and `MAX_FILES`, the threshold is picked rather arbitrarily as different situations require different limits. So it is absolutely acceptable to increase this limit if it is too low for your case.

I agree that a smarter check would be nice. In the not-too-distant future, we plan to revisit all hotspots, so I think that will be a good time as well to try to come up with a smarter solution here than what we currently have - if possible.

---

<div class="post-metadata">

**Author:** ![thorsent](https://avatars.discourse-cdn.com/v4/letter/t/54ee81/32.png) [@thorsent](https://community.sonarsource.com/u/thorsent)\
**Post date:** [August 28, 2023, 1:39pm UTC](https://community.sonarsource.com/t/incorrect-security-remedy-for-adm-zip/97929/6 "2023-08-28T13:39:48Z")

</div>

Thanks for the response, it’s good to know that I didn’t misinterpret the results. Short of reimplementing the approach I think just a comment “Adjust these settings to meet your use case” would be helpful to future implementers.