NPE Blacklisted attachments without extensions

[Zabuzard]

Exception

We just encounted the following exception:

ERROR net.dv8tion.jda.api.JDA - One of the EventListeners had an uncaught exception
java.lang.NullPointerException: Cannot invoke "String.toLowerCase(java.util.Locale)" because the return value of "net.dv8tion.jda.api.entities.Message$Attachment.getFileExtension()" is null
      at org.togetherjava.tjbot.commands.moderation.attachment.BlacklistedAttachmentListener.lambda$doesMessageContainBlacklistedContent$4(BlacklistedAttachmentListener.java:104) ~[classes/

      at java.util.stream.MatchOps$1MatchSink.accept(MatchOps.java:90) ~[?:?]
      at java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1602) ~[?:?]
      at java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:129) ~[?:?]
      at java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:527) ~[?:?]
      at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:513) ~[?:?]
      at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?]
      at java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:230) ~[?:?]
      at java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:196) ~[?:?]
      at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
      at java.util.stream.ReferencePipeline.anyMatch(ReferencePipeline.java:632) ~[?:?]
      at org.togetherjava.tjbot.commands.moderation.attachment.BlacklistedAttachmentListener.doesMessageContainBlacklistedContent(BlacklistedAttachmentListener.java:103) ~[classes/:?]
      at org.togetherjava.tjbot.commands.moderation.attachment.BlacklistedAttachmentListener.onMessageReceived(BlacklistedAttachmentListener.java:44) ~[classes/:?]
      at org.togetherjava.tjbot.commands.system.BotCore.lambda$onMessageReceived$9(BotCore.java:202) ~[classes/:?]
      at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?]
      at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) ~[?:?]
      at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179) ~[?:?]
      at java.util.HashMap$EntrySpliterator.forEachRemaining(HashMap.java:1850) ~[?:?]
      at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?]
      at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?]
      at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
      at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
      at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
      at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) ~[?:?]
      at org.togetherjava.tjbot.commands.system.BotCore.onMessageReceived(BotCore.java:202) ~[classes/:?]
      at net.dv8tion.jda.api.hooks.ListenerAdapter.onEvent(ListenerAdapter.java:403) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at net.dv8tion.jda.api.hooks.InterfacedEventManager.handle(InterfacedEventManager.java:96) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at net.dv8tion.jda.internal.hooks.EventManagerProxy.handleInternally(EventManagerProxy.java:88) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at net.dv8tion.jda.internal.hooks.EventManagerProxy.handle(EventManagerProxy.java:70) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at net.dv8tion.jda.internal.JDAImpl.handleEvent(JDAImpl.java:173) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at net.dv8tion.jda.internal.handle.MessageCreateHandler.handleInternally(MessageCreateHandler.java:136) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at net.dv8tion.jda.internal.handle.SocketHandler.handle(SocketHandler.java:39) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at net.dv8tion.jda.internal.requests.WebSocketClient.onDispatch(WebSocketClient.java:961) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at net.dv8tion.jda.internal.requests.WebSocketClient.onEvent(WebSocketClient.java:847) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at net.dv8tion.jda.internal.requests.WebSocketClient.handleEvent(WebSocketClient.java:825) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at net.dv8tion.jda.internal.requests.WebSocketClient.onBinaryMessage(WebSocketClient.java:1000) ~[JDA-5.0.0-alpha.20.jar:5.0.0-alpha.20]
      at com.neovisionaries.ws.client.ListenerManager.callOnBinaryMessage(ListenerManager.java:385) ~[nv-websocket-client-2.14.jar:?]
      at com.neovisionaries.ws.client.ReadingThread.callOnBinaryMessage(ReadingThread.java:276) ~[nv-websocket-client-2.14.jar:?]
      at com.neovisionaries.ws.client.ReadingThread.handleBinaryFrame(ReadingThread.java:996) ~[nv-websocket-client-2.14.jar:?]
      at com.neovisionaries.ws.client.ReadingThread.handleFrame(ReadingThread.java:755) ~[nv-websocket-client-2.14.jar:?]
      at com.neovisionaries.ws.client.ReadingThread.main(ReadingThread.java:108) ~[nv-websocket-client-2.14.jar:?]
      at com.neovisionaries.ws.client.ReadingThread.runMain(ReadingThread.java:64) ~[nv-websocket-client-2.14.jar:?]
      at com.neovisionaries.ws.client.WebSocketThread.run(WebSocketThread.java:45) ~[nv-websocket-client-2.14.jar:?]

Source

The issue comes from BlacklistedAttachmentListener.java#L107:

Apparently its possible that attachments have no extension, for which our code then fails.

Desired behavior

Files without attachments should be blocked as well.

[Nxllpointer]

Why should files without extension be blocked? Most files without extension are just text anyways. Windows does not run files without extension afaik and linux is file extension independant anyways

[Zabuzard]

Why should files without extension be blocked? Most files without extension are just text anyways. Windows does not run files without extension afaik and linux is file extension independant anyways

I do not care too much. I can see arguments for both sides. For example that such files could be a harmful exe and better safe than sorry.

But I also get that the main argument for this feature is to prevent harmful auto-executables.

@marko-radosavljevic whats your take on this?

[Mom0aut]

If not taken i could easy fix the issue? 😄
duplicate of #618 ?

[Zabuzard]

@Mom0aut yes, please. thank you :)

[Mom0aut]

I would suggest to prevent the uploading of files without extension.

[Nxllpointer]

Do you have a reason for that?

[marko-radosavljevic]

@marko-radosavljevic whats your take on this?

Sorry for the late reply @Zabuzard.

In my opinion, files without extensions should be blocked.

Do you have a reason for that?

So let's examine few aspects: actual threat, perceived threat by the community, moderation overhead and liability & responsibility of the community owners. They would be our risk, and compare it to benefits of allowing files without an extension.

Benefits

Our community does not want, or need, to share files without extensions. It's a very rare occurrence on the server.

Furthermore, files without extensions can't even be opened easily on most systems, so their usability is severely limited.

Risks

Threats

File without an extension can be malware or disturbing/illegal/pornographic media.

Since benefits are non-existent, I don't even have to prove any risks. Because of that, I will skip going over malware attack vectors on different systems, which is a big relief. 😄 It's a massively complex subject, and I don't have enough knowledge to speak with any authority.

We will assume that, generally, windows does not open files without extensions easily in gui. And linux opens files without extensions without any issues, but to run executables you have to explicitly give execute permissions.

Since we are ignoring malware, the threat would be downloading illegal material, or being exposed to NSFW content without your knowledge.

Community

We have a highly technical community, that has a big aversion towards downloading anything from random people on the internet. Rightfully so.

I personally witnessed this many times, helpers that won't download anything, or people questioning files in lobby.

Posting files, especially executables, files with no extension, etc., is perceived as extremely dodgy behaviour. It creates an unsafe atmosphere, and elicits a negative reaction from the community.

This affects everyone. Less experienced members feel unsafe, and security veterans take a principled position even if they how to open the file safely, and they are not in any real danger.

Moderation overhead

Moderators have to make sure members are safe.

To guarantee that, they have to download random files, upload to virustotal or something, and then view the content themselves. They also have to do that safely, which requires knowledge, time and effort.

For example, I mark files I checked with a standard moderator checkmark.

Responsibilities of community owners

Community owners need to make sure they respect the platform they are on. Which in our case means following and enforcing discord tos and community guidelines.

Which, of course, extends also to user uploaded illegal or malicious material. In the end, community owners are responsible for allowing and hosting those files. And the whole community can suffer consequences as the result of that.

Conclusion

Risks outweigh the benefits by a huge margin.

We should aim to remove all files. Word assignment files can have dangerous macros, project archives can be zip bombs, screenshare videos can be NSFW or copyrighted movie that has been illegally downloaded. Project jars can be both malware and illegal material, for eg. keylogger that seeds torrents in the background :p. And finally, files without an extension can be anything.

Instead of blacklist, it's probably much easier and smarter to go with a whitelist. Only things with significant benefits should be allowed, since risks are high by default. And optionally, we can remove that restriction for trusted members (top helpers+ or something similar), if the need arises.

For comparison, we want and need links, benefits are so huge that it's not even worth discussing it. For media embedded in discord, it's something we want, it's safe, and it's easy to moderate. Files are none of that.

[marko-radosavljevic]

Do we agree on converting to whitelist as a solution to this issue?

It's safer that way. I also fear we will continuously discover new problematic extensions, and it's a hassle to add them to the vps.

If so, which extensions will be whitelisted: .txt, .md, .java. Maybe other languages, but we should encourage gists/pastebin anyway?

And of course, extensions discord embeds:

• .jpg .jpeg .JPG .JPEG
• .png .PNG
• .gif and .gifv
• .webm
• .mp4
• .wav
• .mp3 (audio-only mp4s can simply be renamed to embed)
• .ogg

I'm also fine with just merging the current solution. Especially because I'm late to the party, and PR is already done.