You're right on the ground, but I would not fix it by returning a NULL, as zero-length octet/bit strings are valid.

Maybe something as:

  static const char *octet2str(const char *beg, const char *end)
  {
    size_t n = end - beg;
    char *buf = NULL;

    if(n <= (SIZE_T_MAX - 1) / 3) {
      buf = malloc(3 * n + 1);
-     if(buf)
+     if(buf) {
+       *buf = '\0';
        for(n = 0; beg < end; n += 3)
          msnprintf(buf + n, 4, "%02x:", *(const unsigned char *) beg++);
+     }
    }
    return buf;
  }

Sure thing. I replaced malloc with calloc so that everything gets zeroed out:

--- a/lib/x509asn1.c
+++ b/lib/x509asn1.c
@@ -208,11 +208,8 @@ static const char *octet2str(const char *beg, const char *end)
   size_t n = end - beg;
   char *buf = NULL;
 
-  if(!n)
-    return NULL;
-
   if(n <= (SIZE_T_MAX - 1) / 3) {
-    buf = malloc(3 * n + 1);
+    buf = calloc(3 * n + 1, 1);
     if(buf)
       for(n = 0; beg < end; n += 3)
         msnprintf(buf + n, 4, "%02x:", *(const unsigned char *) beg++);

How about this slight tweak, to

1. don't assume the remaining buffer always fits 4 bytes, but use the remainder of the allocation
2. use the actual output size to advance n

  if(n <= (SIZE_T_MAX - 1) / 3) {
    size_t len = 3 * n + 1;
    buf = malloc(len);
    if(buf) {
      *buf = '\0';
      for(n = 0; beg < end;) {
        size_t o = msnprintf(&buf[n], len, "%02x:",
                             *(const unsigned char *) beg++);
        n += o;
        len -= o;
      }
    }
  }

How about this slight tweak, to...

I don't think it's very useful: each msnprintf call stores exactly 4 bytes: 2 hex digits, a colon and the null terminator unless it is broken. Anyway, if it complies, the result is the same.
in addition, we would be in a very bad condition in highly improbable cases where msnprintf returns -1 !

Sure, my version is just slightly more "defensive" and doesn't assume as much, like that the remaining allocation always fit 4 bytes. I won't insist, but I think that's a better way to write this code.

msnprintf() cannot return -1.

... like that the remaining allocation always fit 4 bytes.

It does: the source of allocation size is the same as the loop count control and overflow cannot occur thanks to the SIZE_T_MAX test.
I also consider the current code involving as much constants as possible more readable, but that's a personal sentiment.

my version is just slightly more "defensive" and doesn't assume as much

If it makes you feel safer, we could also use a dynbuf now that they exist.

we could also use a dynbuf now that they exist.

Oh right! Yes I would approve of that.

Something like:

#include "dynbuf.h"
.
.
.
static const char *octet2str(const char *beg, const char *end)
{
  struct dynbuf buf;
  CURLcode result;

  Curl_dyn_init(&buf, 3 * CURL_ASN1_MAX + 1);
  for(result = Curl_dyn_addn(&buf, "", 0); !result && beg < end; beg++)
    result = Curl_dyn_addf(&buf, "%02x:", *(const unsigned char *) beg);
  return Curl_dyn_ptr(&buf);
}

@z2-2z : Do you want to create a commit for it or would you prefer I do it ?

The typecast can be removed too now, right? Like:

    result = Curl_dyn_addf(&buf, "%02x:", beg[0]);

The typecast can be removed too now, right?

Don't you fear a possible sign extension?

The dynbuf code is now included in the pull request

Without the cast, '\xb4' will be output as ffffffb4 ...

whoopsie, fixed now

Thanks!