Vulnerabilities found in code

Question

Hi,

Our Information Security team have found the following concerns in the code of the PlantUML library. Are there any plans for these to be address?

Thanks

Title	Message
MessageDigest msgDigest = MessageDigest.getInstance("MD5");	Detected MD5 hash algorithm which is considered insecure. MD5 is not collision resistant and is therefore not suitable as a cryptographic signature. Use HMAC instead.
ServerSocket ss = new ServerSocket(this.port);	Detected use of a Java socket that is not encrypted. As a result, the traffic could be read by an attacker intercepting the network traffic. Use an SSLSocket created by 'SSLSocketFactory' or 'SSLServerSocketFactory' instead.
Socket soc = new Socket(this.ipClient, this.port);	Detected use of a Java socket that is not encrypted. As a result, the traffic could be read by an attacker intercepting the network traffic. Use an SSLSocket created by 'SSLSocketFactory' or 'SSLServerSocketFactory' instead.
ServerSocket s = new ServerSocket(this.listenPort);	Detected use of a Java socket that is not encrypted. As a result, the traffic could be read by an attacker intercepting the network traffic. Use an SSLSocket created by 'SSLSocketFactory' or 'SSLServerSocketFactory' instead.
((Statement)statements.get(thisStatement)).execute();	Detected a formatted string in a SQL statement. This could lead to SQL injection if variables in the SQL statement are not properly sanitized. Use a prepared statements (java.sql.PreparedStatement) instead. You can obtain a PreparedStatement using 'connection.prepareStatement'.
ServerSocket serverConnect = new ServerSocket(port, 50, bindAddress1);	Detected use of a Java socket that is not encrypted. As a result, the traffic could be read by an attacker intercepting the network traffic. Use an SSLSocket created by 'SSLSocketFactory' or 'SSLServerSocketFactory' instead.
ServerSocket server = new ServerSocket(4242);	Detected use of a Java socket that is not encrypted. As a result, the traffic could be read by an attacker intercepting the network traffic. Use an SSLSocket created by 'SSLSocketFactory' or 'SSLServerSocketFactory' instead.

Comments

(I am not a plantuml developer)
Wow! Well done for getting the code tested. I would point out that MD5 is fine if it's not being used in cryptographic operations, for example to look for 'changes' to a diagram.

Also, I'm not sure PlantUML has any database connectivity, so is ((Statement)statements.get(thisStatement)).execute(); actually a SQL statement or is your security team just flagging some code with a regular expression that looks like it might be a SQL statement?
Again, depending on context, even though I would agree a prepared statement is best practice, if there are no user-provided values, it's not a problem.
Genuinely interested if there is some random DB query kicking around!

Finally, the non SSL sockets should be fine if you're not planning on sending any confidential data to a running plantuml server (if you're just using it as a library for example when generating documentation from code on a build server).
From these points, I would say this is probably the output of what's called 'static analysis', which may generate some false positives.
I'd have a chat with your security team and development teams, feel free to show them this comment, and see if they think the Security issues are 'blocker' or 'acceptable' issues.
They might need to review the code in more detail - people here might also do that, but you've not provided the file and line numbers for the code you've called out so I can't really look!
Hope this is helpful :)

---

Hi Chris,

Appreciate the detailed reply!

As you can probably guess, I'm not a developer either. This has been flagged in the latest release of "PlantUML for Confluence". It's a free third party application which uses the plantUML library so when I flagged to their developer he advised it would be the plantUML library where any changes would be required. They must just package up the latest version in the app.

App can be found in the Atlassian Marketplace: https://marketplace.atlassian.com/apps/41025/plantuml-for-confluence?hosting=datacenter&tab=overview

I think your assumption of analysis is correct, the security team have decompiled the app and then ran the code against a database of vulnerabilities (as I understand it). I'll see if they can provide the line numbers for these.

Answer 1

Thank you for reviewing our code!

As the lead developer of PlantUML, I'd like to provide you with some insights.

Regarding MD5:
In fact, the use of MD5 hashing in our system is not for cryptographic purposes, but rather for generating internal identifiers. For instance, we use it to create the ETag signature of a diagram. Even in the event of a hash collision, the only consequence would be cache invalidation, which is not a significant issue.

Regarding ServerSocket:
The core PlantUML library encompasses a wide range of features to facilitate seamless integration with other products. For example, it includes a simulated FTP server (https://plantuml.com/ftp) that, while mimicking a real server, actually generates diagrams. It also supports Telnet connections and even has a micro web server (https://plantuml.com/picoweb). These connection methods are intended for local use and are therefore not secured. However, it's important to note that this code is not utilized in the Atlassian context.

Please let us know if your security team requires any further information.

Comments

((Statement)statements.get(thisStatement)).execute();    Detected a formatted string in a SQL statement. This could lead to SQL injection if variables in the SQL statement are not properly sanitized. Use a prepared statements (java.sql.PreparedStatement) instead. You can obtain a PreparedStatement using 'connection.prepareStatement'.

By the way, I don't think this is present in our library

---

Appriciate you looking into this and providing a detailed reply. I will pass the comment regarding the SQL statements back to our security team to clarify the location. I presume it is somewhere within the Confluence app rather than the plantUML library in this case.

---

Hi plantUML,

FYI, the statements vulnerability was in net/sourceforge/plantuml/jasic/Jasic.java but can see that has been removed in the latest release. Also, looking into this myself I don't see any relation to SQL statements, so looks to be a false positive and have therefore fed this back to our security team.

Many thanks