Use of string instead of byte[]

[akwick]

During an empirical study to understand the nature of cryptographic misuses in enterprise-driven projects on GitHub, we randomly inspected a few of the misuses. One of the misuses for which we could confirm as a true positive of the analysis, CogniCryptSAST, is in this project.

• In the class ninja.AwsLegacyHashCalculator (L. 113) a string is passed as a secret key and that is considered insecure. In Java, strings are immutable and stay in memory until collected by Java's garbage collector. Thus, they are longer visible in memory for attackers than necessary and outside of the direct control of the developer. The suggested data types by the JCA are bytes.

[jakobvogel]

Hallo @akwick 👋 Wow. Krass. 🤩

First of all, many thanks for this. We really appreciate your efforts, both for our product here, and in a wider sense, as a company relying on secure products running in a secure environment.

Having said that, we are not going to fix this particular line. 🙃 (Yeah, I know, strange twist. 😅) You see, S3 Ninja is a development tool designed to serve as a mockup during development of AWS S3 connected software. It never (really, never) should be used to store any real data, for a wide range of reasons, including security, reliability, performance, scalability, etc. (Rather the lack thereof.) The password, in fact, is in a plain-text file in this very repository, and an attacker will have access to that file long before getting even close to the memory controlled by the JVM. The sole purpose of S3 Ninja is to save some bucks during development. It never should be exposed to anyone outside of a LAN/VPN. Ideally, it should only run as a Docker container on the developer's computer, and it only should contain funny pictures of cats & dogs downloaded from the internet and transmitted for testing purposes. 😉

However, in general, your point is very valid, of course. If anyone reading this wants to submit a PR demonstrating the proper use of byte arrays instead of strings, we will happily accept it. For now, if you want, you could also add a comment in the file, warning people against blindly copying this piece of code. If you want to document your finding yourself (also in the sense of getting the "contributor" tag on GH), please submit a PR. Else, I will add a respective comment myself. Would that be ok?

Herzlichen Dank nochmals. Wir sind alle hin und weg von der Aktion. Großartig. Wirklich krass. 😊

[jakobvogel]

By the way, your point causes us to cross-check our production libraries and non-public code for similar issues. So, maybe, it still will turn out to have positive security effects behind the scenes. 😉

[akwick]

Thanks for your friendly response! 🙏 I hope for your software quality that you did not find any security issues within your production libraries or public code. As you may see I created a pull request with the comment you suggested above. 😄

[jakobvogel]

Many thanks again, @akwick. Keep up the great work!