280051 - Crash ATOB_AsciiToData( (const char*)inbuff, &inlen64 ) when passing an empty string !

Status: NEW

(NSS :: Libraries, defect, P5)

Description

[jorelia ANDRE]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040206 Firefox/0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040206 Firefox/0.8

So here is my code.

It is very simple, the function crash when you pass an empty string.

Function should be protected, if there is nothing to decode or encode, the
function must return null or the string parameter itself, up to you.

Have a good time.

Jorelia, (below my code).



			const char* inbuff = inbuf.c_str();

			// the base64 decoded string
			unsigned int inlen64;
			unsigned char* inbuf64 = ATOB_AsciiToData( (const char*)inbuff, &inlen64 );

			// set the size of the output buffer
			outbuf.resize( inlen64+32, '\0' );
			const char* outbuff = outbuf.c_str();
			int outlen = outbuf.size();


============================================================
Oh, I would just take the opportunity to say that I'm using the functions
 - PR_GetErrorTextLength()
to allocate the space for returned buffer 
and 
 - PR_GetErrorText
but return the function return nothing. How and When to use those function.
Could you tell us that in the documentation ???

EVEN IF PR_GetError() return an error.




	void DESAlgorithm::addErrorMsg( void )
	{
		int ilen = PR_GetErrorTextLength();

		if ( ilen > 0 )
		{
			char *text = (char*)calloc( (size_t) ilen, sizeof(char) );
			if ( text )
			{
				m_ErrorMsg += "\n";

				PR_GetErrorText( text );

				m_ErrorMsg += text;
				
				free( text ); 

				return;
			}
		}
	}


Reproducible: Always

Actual Results:  
Crash, memory violation.

Expected Results:  
Function should be protected, if there is nothing to decode or encode, the
function must return null or the string parameter itself, up to you.

Comment 1

[Julien Pierre]

I looked at the code and I can see that ATOB_AsciiToData will crash if you pass a NULL string as input. I'm afraid that many NSS functions are similarly unforgiving. Marking CONFIRMED.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Please see bug 346583 for a lengthy discussion of this topic.
There have been several patches written for that bug, all rejected, 
proving that this is trickier to get right than it looks.

This bug is not strictly a duplicate of that other one because the two 
report the same problem with different functions in the same file.
They might have the same underlying cause.

In any case, I'd like to see the two bugs combined and a single patch
written that fixes all the issues in both bugs.

Comment 3

[Julien Pierre]

The crash on NULL argument in ATOB_AsciiToData is because we call PORT_Strlen on the input string. This is a different bug than 346583 . But they could be consolidated.