struct rsa_cert {
    u32 key_id;
    char rsa_signature[1024];
    char metadata[32];
    char content_hash[20];
};

int verify_cert (struct rsa_cert cert) {
  char *cert_hash=SHA1(cert.metadata + cert.content_hash);
  char *sig_hash=rsa_decrypt(cert.rsa_signature, cert.key_id);

  if (strncmp(cert_hash, sig_hash, SHA1_LENGTH) == 0) {
     return CERT_OK;
  } else {
     return CERT_BAD;
  }
}

int is_a_valid_disc(struct rsa_cert cert, char *disc_hash) {
    if(memcmp(disc_hash, cert.content_hash, SHA1_LENGTH) != 0)  {
      return DISC_BAD;
    }

    if(verify_cert (cert) == CERT_BAD) {
      return DISC_BAD;
    } else {
      return DISC_OK;
    }

}
 

 http://wiibrew.org/index.php?title=Signing_bug

Um... that's not a WTF - it's a bug, probably caused by insufficient coffee.

char metadata[32];
char content_hash[20];

[...]

char *cert_hash=SHA1(cert.metadata + cert.content_hash);

EDIT: oh, pointer math, never mind. 

Zecc:

char metadata[32];
char content_hash[20];

[...]

char *cert_hash=SHA1(cert.metadata + cert.content_hash);

Ehmm... What?

 

EDIT: oh, pointer math, never mind. 

Spectre:

Zecc:

char metadata[32];
char content_hash[20];

[...]

char *cert_hash=SHA1(cert.metadata + cert.content_hash);

Ehmm... What?

 

EDIT: oh, pointer math, never mind. 

Pointer + pointer? Mind.

Pointer + pointer illegal in C? 

fist-poster:

Pointer + pointer illegal in C? 

Lingerance:

fist-poster:

Pointer + pointer illegal in C? 

It's perfectly legal. It's never wise unless you know where every single other piece of reserved memory is so you know the resulting pointer is someplace safe.

 Not according to MinGW.

int main() {
   char* a, *b, *c;
   c = a + b;
   return 0;
}

 error: invalid operands of types 'char*' and 'char*' to binary 'operator+'

c = a - (char*)(-(long long)b); // Legal C, illegal C++

Lingerance:

fist-poster:

Pointer + pointer illegal in C? 

It's perfectly legal.

C99 Draft (N869, 18 January, 1999):

6.5.6 Additive operators

[...] 

#2

For addition, either both operands shall have arithmetic type, or one operand shall be a pointer to an object type and the other shall have integer type. (Incrementing is equivalent to adding 1.)

#3

For subtraction, one of the following shall hold:

-- both operands have arithmetic type;

-- both operands are pointers to qualified or unqualified versions of compatible object types; or

-- the left operand is a pointer to an object type and the right operand has integer type.

The article referred to by the OP clearly states it is pseudocode, so it doesn't really matter whether or not you can do it.

Most likely, the + is used to show concatenation, considering the kind of data the operands represent. I haven't explored the Wii firmware enough to know, though.

Pidgeot:

The article referred to by the OP clearly states it is pseudocode, so it doesn't really matter whether or not you can do it.

Most likely, the + is used to show concatenation, considering the kind of data the operands represent. I haven't explored the Wii firmware enough to know, though.

Bingo.  The + is for concatentation, the example was pseudocode, the args aren't even right for SHA1() and you cannot add pointer types in C without casting one to an integral type which is just stupid. 

morbiuswilters:

and you cannot add pointer types in C without casting one to an integral type which is just stupid. 

PJH:

morbiuswilters:

and you cannot add pointer types in C without casting one to an integral type which is just stupid. 

I'm intrigued - could you give a contrived example of when adding two pointers would yield a sensible result?

Hmm, perhaps you are misunderstanding me.  I'm saying it would be stupid to add pointers and that you have to cast to an int just to get the compiler to accept it.  I'm not saying it's stupid that it is  illegal C.  I really cannot even think of a contrived example where you would want to do this, but I have faith that somebody will manage to come up with.

Now, pointer multiplication, that is absolutely essential in my day-to-day work.

pOffset = (char *)((unsigned long)pBuf1 * (unsigned long)pBuf2); 

Pidgeot:

The article referred to by the OP clearly states it is pseudocode

Didn't anybody see that the code uses strncmp to compare two binary buffers?

Didn't anybody see that the code uses strncmp to compare two binary buffers?

TFA:

The bug here is that the hash can (and very likely does) contain a NULL byte, (chr)0 that is. To quote from the first google hit for strncmp: "Compares up to num characters of the C string str1 to those of the C string str2. This function starts comparing the first character of each string. If they are equal to each other, it continues with the following pairs until the characters differ, until a terminating null-character is reached, or until num characters match in both strings, whichever happens first." This last part means that if it finds a NULL byte, it stops comparing, even if there is more data after the NULL.

alegr:

Didn't anybody see that the code uses strncmp to compare two binary buffers?

Yes, because I read TFA. That's what the OP was getting at as the real WTF.

Of course this has been fixed, complete with a rant against Datel for using the vuln (Basically, by using it in a commercial product, Datel guaranteed Nintendo would fix it.)

The posters of TFA basically said "yeah we knew about it, we were using it to get into the system."  Grey-hats all around there. :) 

vt_mruhlin:

Didn't anybody see that the code uses strncmp to compare two binary buffers?

Those of us that went to the actual article did:

TFA:

The bug here is that the hash can (and very likely does) contain a NULL byte, (chr)0 that is.

TRWTF then is the poor pseudo-coding.  If "+" can mean "concatenate byte arrays", why shouldn't we assume that assigning the returned hash to a char* pointer won't invoke a conversion operator that turns it into an ASCII hex string?  The FA's author needs to grok that if you're criticising someone for being incorrect, it behooves you to be fairly damn precise about it.

DaveK:

TRWTF then is the poor pseudo-coding.  If "+" can mean "concatenate byte arrays", why shouldn't we assume that assigning the returned hash to a char* pointer won't invoke a conversion operator that turns it into an ASCII hex string?  The FA's author needs to grok that if you're criticising someone for being incorrect, it behooves you to be fairly damn precise about it.

It was clearly mentioned in the article that this was pseudocode, so you should not go in expecting it to be absolutely precise.  Additionaly, the actual bug was explicitly pointed out in the explanation, meaning you didn't need to read the article at all.  I suppose it made me stop for a second and wonder what the hell the code was trying to do, but concatenation was the only thing that made logical sense.  The '+' operator is used in many languages for string concatentation and since I knew I was reading pseudocode and that the pointers couldn't be added, I came to the only logical conclusion..

vt_mruhlin:

Didn't anybody see that the code uses strncmp to compare two binary buffers?

Those of us that went to the actual article did:

TFA:

The bug here is that the hash can (and very likely does) contain a NULL byte, (chr)0 that is. To quote from the first google hit for strncmp: "Compares up to num characters of the C string str1 to those of the C string str2. This function starts comparing the first character of each string. If they are equal to each other, it continues with the following pairs until the characters differ, until a terminating null-character is reached, or until num characters match in both strings, whichever happens first." This last part means that if it finds a NULL byte, it stops comparing, even if there is more data after the NULL.

It's still an improvement over Microsoft, who used RC4 to compute a hash over the Xbox's bootloader -- which would have been clever, if not for the fact that RC4 can't actually be used to compute hashes.

After discovering their mistake, Microsoft switched to TEA for computing the bootloader's hash in the next Xbox revision.  Can you guess what TEA isn't supposed to be used for?

morbiuswilters:

It was clearly mentioned in the article that this was pseudocode, so you should not go in expecting it to be absolutely precise. 

Moreso, if it's pointing out a bug in actually using strncmp() instead of memcmp().