Last Comment Bug 287112 - atob() on a string with a '\n' throws OOM exception
: atob() on a string with a '\n' throws OOM exception
Status: RESOLVED FIXED
: helpwanted
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
: Hixie (not reading bugmail)
Mentors:
javascript:try{alert(atob("foo\npy"))...
: 408162 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-03-21 13:35 PST by Christopher Aillon (sabbatical, not receiving bugmail)
Modified: 2007-12-13 06:47 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Wrong diff. (2.02 KB, patch)
2007-01-19 17:46 PST, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Review
Fix. (1.58 KB, patch)
2007-01-19 17:48 PST, Johnny Stenback (:jst, jst@mozilla.com)
jonas: review+
jonas: superreview+
Details | Diff | Review
Fix that landed. (1.87 KB, patch)
2007-01-22 16:48 PST, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Review

Description Christopher Aillon (sabbatical, not receiving bugmail) 2005-03-21 13:35:44 PST
Not sure what the correct behavior is, but it should not claim to be OOM.
Comment 1 Alex Vincent [:WeirdAl] 2005-03-21 15:18:30 PST
nsGlobalWindow::Atob
http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#4019

codetovalue
http://lxr.mozilla.org/seamonkey/source/nsprpub/lib/libc/src/base64.c#172

codetovalue, passed \n, will result in -1 returned to decode4to3 (next function
in base64.c, line 216).

Because of this, decode4to3 assumes failure, returns it, and everything
generally unravels.

>	plc4.dll!codetovalue(unsigned char c='
')  Line 176	C
 	plc4.dll!decode4to3(const unsigned char * src=0x04157790, unsigned char *
dest=0x041577c8)  Line 216 + 0xe	C
 	plc4.dll!decode(const unsigned char * src=0x04157790, unsigned int
srclen=0x00000006, unsigned char * dest=0x041577c8)  Line 323 + 0xd	C
 	plc4.dll!PL_Base64Decode(const char * src=0x04157790, unsigned int
srclen=0x00000006, char * dest=0x041577c8)  Line 416 + 0x11	C
 	gklayout.dll!nsGlobalWindow::Atob(const nsAString & aAsciiBase64String={...},
nsAString & aBinaryData={...})  Line 4016 + 0x15	C++
Comment 2 Boris Zbarsky [:bz] 2005-04-13 00:21:33 PDT
So the basic problem is that PL_Base64Decode does:

  A null is retuned if the allocation fails, or if the source is not well-coded.

So from the caller, all we know is that one of those two happened.  Given that,
perhaps NS_ERROR_FAILURE is a more appropriate error to throw?
Comment 3 Hellekin 2005-07-22 12:55:26 PDT
Actually, atob() fails, period:
javascript:try{alert(atob("foo"));}catch(e){alert(e)}
Comment 4 Hellekin 2005-07-22 15:25:26 PDT
From: http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#4084

4084 nsGlobalWindow::Atob(const nsAString& aAsciiBase64String,
4085                      nsAString& aBinaryData)
4086 {
4087   aBinaryData.Truncate();
4088 
4089   if (!Is8bit(aAsciiBase64String)) {
4090     return NS_ERROR_DOM_INVALID_CHARACTER_ERR;
4091   }
4092 
4093   char *base64 = ToNewCString(aAsciiBase64String);
4094   if (!base64) {
4095     return NS_ERROR_OUT_OF_MEMORY;
4096   }
4097 

Here, ToNewCString returns false on a plaintext string ?

Actually, there are two definitions of ToNewCString() in 
The second one skips the sting conversion.

See lines 316  and 355 in :
http://lxr.mozilla.org/seamonkey/source/xpcom/string/src/nsReadableUtils.cpp



4098   PRInt32 dataLen = aAsciiBase64String.Length();
4099 
4100   PRInt32 resultLen = dataLen;
4101   if (base64[dataLen - 1] == '=') {
4102     if (base64[dataLen - 2] == '=') {
4103       resultLen = dataLen - 2;
4104     } else {
4105       resultLen = dataLen - 1;
4106     }
4107   }
4108 
4109   resultLen = ((resultLen * 3) / 4);
4110 
4111   char *bin_data = PL_Base64Decode(base64, aAsciiBase64String.Length(),
4112                                    nsnull);
4113   if (!bin_data) {
4114     nsMemory::Free(base64);
4115 
4116     return NS_ERROR_OUT_OF_MEMORY;
4117   }
4118 
4119   CopyASCIItoUCS2(Substring(bin_data, bin_data + resultLen), aBinaryData);
4120 
4121   nsMemory::Free(base64);
4122   PR_Free(bin_data);
4123 
4124   return NS_OK;
4125 }
Comment 5 Christian :Biesinger (don't email me, ping me on IRC) 2005-07-22 17:16:08 PDT
Hellekin: The problem is that PL_Base64Decode returns null both for OOM and if
the input string is not valid base64.
Comment 6 Yuh-Ruey Chen 2005-10-07 06:33:13 PDT
(In reply to comment #3)
> Actually, atob() fails, period:
> javascript:try{alert(atob("foo"));}catch(e){alert(e)}

atob('foo') doesn't throw an error on my build (Gecko/20051006 Firefox/1.4.1).

But I've found lots of other strings that throw the OOM error. Any 1-length
string throws the error. Any string containing a character that's not an
alphanumerical character or '+' or '/' throw the error. So yeah, the passed
string needs to be in base64 (which of course makes it useless).
Comment 7 Yuh-Ruey Chen 2005-10-08 06:27:44 PDT
atob is not "useless" - it was a misunderstanding on my part.

Email correspondence w/ Christian Biesinger:
> > > ? Why does it make that useless? The entire point of atob is to convert 
> > > base64 to the original data...
> > 
> > I was under the impression that btoa was the one that converts base64 to 
> > the original data. I've never used atob and btoa tho (except when 
> > testing this bug), so am I mistaken?
> 
> Yes - the "b" stands for "binary", the "a" for ASCII. Therefore, the "a" 
> actually means "base64". Yeah, this is somewhat confusing...

This really needs to be documented on devmo. Google search on atob and btoa was
fruitless...
Comment 8 Wladimir Palant 2007-01-19 07:26:19 PST
I think this should be fixed. Getting an out of memory error for a bad base64 string is very confusing. When I saw that I had to dig through LXR to understand what was really going on.
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-19 17:46:32 PST
Created attachment 252123 [details] [diff] [review]
Wrong diff.
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-19 17:48:08 PST
Created attachment 252124 [details] [diff] [review]
Fix.
Comment 11 Jonas Sicking (:sicking) 2007-01-19 17:53:55 PST
Comment on attachment 252124 [details] [diff] [review]
Fix.

I take it that PL_Base64Decode never goes over resultLen characters.

Given that's ensured, r/sr=sicking
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-22 16:48:33 PST
Created attachment 252403 [details] [diff] [review]
Fix that landed.
Comment 13 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-22 16:49:00 PST
Fixed on the trunk.
Comment 14 Dave Townsend [:mossop] 2007-12-13 06:47:13 PST
*** Bug 408162 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.