Check that handler->isremote has been set before calling it.

[daviesrob]

Failing to check this could cause an attempt to call a NULL pointer, where the plugin doesn't supply an isremote() implementation.

Credit to OSS-Fuzz Fixes oss-fuzz 67349

[jmarshall]

The protocol as designed is that plugins must always supply the open and isremote methods, and if priority >= 2000 they may also supply the vopen method.

So just like it doesn't check that handler->open is non-NULL or that the hFILE_backend methods (other than flush) are non-NULL before it calls them, checking here isn't really warranted either.

Presumably the problem detected by OSS-Fuzz was that hfile_plugin_init_crypt4gh_needed() has this as NULL. This is the bug; it should just set it to hfile_always_local() instead.

There is also a bug in hfile_plugin_init_mem(), which doesn't set the open method. It's under-documented, but presumably hopen("mem:", "r:", …) is intended to be always used with : in the mode string so that the vopen method will be used. IMHO crashing on hopen("mem:", "r") is borderline (a fuzz tester would probably agree!) and it would be better to set a dummy open method that always sets EINVAL and fails.

To ensure that such problems don't occur, hfile_add_scheme_handler() could validate that handlers do implement the required methods, e.g.,

--- a/hfile.c
+++ b/hfile.c
@@ -1022,6 +1022,10 @@ void hfile_add_scheme_handler(const char *scheme,
                               const struct hFILE_scheme_handler *handler)
 {
     int absent;
+    if (handler->open == NULL || handler->isremote == NULL) {
+        hts_log_warning("Couldn't register scheme handler for %s: missing method", scheme);
+        return;
+    }
     if (!schemes) {
         if (try_exe_add_scheme_handler(scheme, handler) != 0) {
             hts_log_warning("Couldn't register scheme handler for %s", scheme);

[daviesrob]

Agreed, while a bit more complicated, checking in hts_add_scheme_handler() is probably better. And good spot on mem missing the open interface. I have pushed a revised version which works as suggested.

As an aside, there's a bit of a muddle with the functions used to handle data: and mem: URIs. I'm tempted to rename some (in a separate pull request) to make what they're used for clearer.