229374 - more to do for bug #157644...

Status: RESOLVED FIXED

Description

[(not reading, please use [e-mail] instead)]

more to do for bug #157644...

dan got email from [e-mail], pointing out that the fix for #157644
plugged one security hole, but not them all.

from the reporter:

the correct fix is to limit the number of messages to (MAXINT(sizeof(Pop3MsgInfo))

if an evil server sends a larger number of messages, we'll only allocate space
for 50k.  but if part way through the list, if the server introduces a message
that is < than the max but > 50k, we'll allocate more space.

SendUidl() doesn't bounds check on the 50k message limit.

the patch in bug #157644 assumes that message numbers are sequential.

I've got the complete email from zen-parse.

Comment 1

[(not reading, please use [e-mail] instead)]

giving zen-parse ([e-mail]) access to this new bug.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

As far as I can tell, this is our only open bug which would allow a remote
server to take control of a Mozilla session. 'twould be nice if we could fix it :-)

Comment 3

[chris hofmann]

putting on the 1.7 radar to see if we can get a patch.

Comment 4

[David :Bienvenu]

Created attachment 148899 [details] [diff] [review]
proposed fix

Comment 5

[David :Bienvenu]

Comment on attachment 148899 [details] [diff] [review]
proposed fix

this was the fix I proposed all along...

Comment 6

[(not reading, please use [e-mail] instead)]

Comment on attachment 148899 [details] [diff] [review]
proposed fix

r/a=sspitzer

I'm not sure why we just didn't do what david suggested.

david, should we back out http://bugzilla.mozilla.org/show_bug.cgi?id=157644

Comment 7

[David :Bienvenu]

yes, we should back it out, just to remove the unneeded code and simplify it.

Comment 8

[Daniel Veditz [:dveditz]]

Fix got approvals on 5/19, is it checked in?

Comment 9

[Scott MacGregor]

I think your patch is short a parentheses :)

Comment 10

[David :Bienvenu]

fixed on branch, with added parenthesis

Comment 11

[David :Bienvenu]

or not - new cvsisn't working for this tree...

Comment 12

[chris hofmann]

cleaning up 1.7 bug lists -- is this bug ready to be marked fixed?

Comment 13

[(not reading, please use [e-mail] instead)]

over to david who has the fix and is going to land on trunk (he already landed
on the branch).

I'll log a bug about backing out bug #157644.

note, if we need to test this we can use servterm
http://www.snapfiles.com/get/servterm.html to emulate an evil pop server.

Comment 14

[David :Bienvenu]

fixed on trunk.

Comment 15

[(not reading, please use [e-mail] instead)]

backing out the fix for bug #157644 is covered by bug #245066

Comment 16

[Daniel Veditz [:dveditz]]

Adding Jon Granrose to CC list to help round up QA resources for verification

Comment 17

[Jon Granrose]

adding karen to verify on the 1.7 branch

Comment 18

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 148899 [details] [diff] [review]
proposed fix

a=blizzard for 1.4.3

Comment 19

[Christopher Aillon (sabbatical, not receiving bugmail)]

And checked in.

Comment 20

[Daniel Veditz [:dveditz]]

Removing security-sensitive flag for bugs on the known-vulnerabilities list

Comment 21

[Karen Huang]

Since David mentioned that this bug need to be verified in the debugger, by
tweaking some values at runtime...
I had requested Seth to help for verifying this bug for 1.7....

Comment 22

[Mark Cox]

Note: The Common Vulnerabilities and Exposures project (cve.mitre.org) has
assigned the name CAN-2004-0757 to this issue.