arangodb-java-driver icon indicating copy to clipboard operation
arangodb-java-driver copied to clipboard

passwords should _not_ use `String` type

Open nkiesel opened this issue 3 years ago • 4 comments

Storing / passing passwords as String is a common security issue because these password strings will remain in the common String pool for a long time. Instead, passwords should use char[] or byte[] as types. I see this mistake in quite a few places in the API, but it all starts at com.arangodb.ArangoDB.Builder#password

nkiesel avatar Jun 07 '21 17:06 nkiesel

Hello @rashtao how you doing? Is this issue still open? Can I help you to fix? I would like to contribute to this project

brunobarros2093 avatar Jun 03 '23 17:06 brunobarros2093

Hi @obrunojava , thanks for your offer!

In my opinion, the problem cannot be easily fixed, because in the HttpConnection (https://github.com/arangodb/arangodb-java-driver/blob/main/http/src/main/java/com/arangodb/http/HttpConnection.java) the username and password are converted to an Authorization header value and this can only be set in the underlying Vert.x HttpRequest as String, see

https://github.com/arangodb/arangodb-java-driver/blob/2ad5326ecb04db261f2dc352a6092b77044f9789/http/src/main/java/com/arangodb/http/HttpConnection.java#L255

and

https://github.com/vert-x3/vertx-web/blob/4.4/vertx-web-client/src/main/java/io/vertx/ext/web/client/HttpRequest.java#L184

Therefore the string will be anyways interned there. In this case the string it is {user}:{passwd} Base64 encoded, not plain text, but still a vulnerability.

How would you suggest addressing it?

rashtao avatar Jun 07 '23 11:06 rashtao

@nkiesel Is this really an issue? String literals are put into the pool by the compiler but instances created by a constructor call are not, as far as I know:

assertTrue("abc" == "abc");
assertFalse(new String("abc") == new String("abc"));

Other types use a pool also for other values like Integer.valueOf() but not String except you are using intern() to force a pool instance.

aburmeis avatar Jan 09 '24 14:01 aburmeis

Even if we could avoid interning the password string, this will be effectively a long-living String in the heap, living until the driver will be used, because we need to send this information along with every request. Or instead of it we would have in the heap the entire HTTP Authorization header string.

From a security standpoint, in my opinion these are vulnerable in the same way to memory dumps attacks. Long-living char[] or byte[] would be vulnerable as well.

rashtao avatar Jan 10 '24 10:01 rashtao