bennadel / JSONWebTokens.cfc

Thi is a ColdFusion gateway to help encode and decode JSON web tokens.
Other
59 stars 26 forks source link

Url base 64 is broken #6

Open lud-gfi opened 7 years ago

lud-gfi commented 7 years ago

Hello,

I ported the code to an old version of CFML. I don't know if you face this problem, but the padding function was broken :

    function convertToBase64( input ) {
        input = replace( input, "-", "+", "all" );
        input = replace( input, "_", "/", "all" );
        var paddingLength = ( 4 - ( len( input ) mod 4 ) );
        return( input & repeatString( "=", paddingLength ) );
    }

When the input length is a multiple of 4, the modulo yields 0, so 4 - 0 yields 4, and we have a useless padding ==== added to the string.

If figured out with this simple payload : 'payload' (i.e. the simple word "payload" as a string. The JSON version ("payload") will be transformed to "payload when decoding (so there is a missing quote), and the JSON unserialize will just yield null.

I'm not 100% sure wether this behaviour is because of the unwanted padding, but the problem disappears with this dumb version of the function :

    function convertToBase64( input ) {
        input = replace( input, "-", "+", "all" );
        input = replace( input, "_", "/", "all" );
        switch(len( input ) mod 4) {
            case 1: return input & '===';
            case 2: return input & '==';
            case 3: return input & '=';
            default: return input;
        }
    }

I will not make a pull request because of the old version of the code we are using and because I'm not sure of what is the deep truth about this :)

Cheers.

dpbabcock commented 4 years ago

I just "discovered" this same issue. That will teach me to read the issues and not just blindly assume all is well ;-). I discovered mine when using this code to decode a REAL JWT returned from Azure ID services. The header segment of the JWT needs to be decoded to get the "kid" value to get the correct public key certificate from microsoft for subsequent verification of the JWT. The value returned in my case turns out to be a multiple of 4 so it provides the redundant padding. as above. My fix was slightly different but has the same effect:

public string function convertToBase64( required string input ) {
    input = replace( input, "-", "+", "all" );
    input = replace( input, "_", "/", "all" );
    var paddingLength = ( 4 - ( len( input ) % 4 ) );
    if (paddingLength EQ 4) {
                  paddingLength = 0;
            }
    return( input & repeatString( "=", paddingLength ) );
   }

Now that I look at it, I like the following even better:

public string function convertToBase64( required string input ) {
    input = replace( input, "-", "+", "all" );
    input = replace( input, "_", "/", "all" );
   // if the input is a multiple of 4, no padding is needed. Simply return it.
           if (len(input) % 4 EQ 0)  {
              return input;
           }
   // otherwise pad the input with "=" until it is a multiple of four.
    var paddingLength = ( 4 - ( len( input ) % 4 ) );
    return( input & repeatString( "=", paddingLength ) );
}