Closed Bug 533647 Opened 10 years ago Closed 10 years ago

Memory corruption vulnerability in tools/codesighs/codesighs.c

Categories

(Firefox Build System :: General, defect, critical)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: reed, Assigned: dbaron)

References

()

Details

(Whiteboard: [sg:moderate local])

Attachments

(2 files)

Received a security vulnerability notice to security@ from Jeremy Brown regarding the codesighs (https://wiki.mozilla.org/Codesighs) code:

257	    while(0 == retval && NULL != fgets(lineBuffer,
sizeof(lineBuffer), inOptions->mInput))
(gdb)
259	        trimWhite(lineBuffer);
(gdb)
trimWhite (inString=0xbfffd310 "1\tCODE\t", 'A' <repeats 15 times>,
"\t", 'A' <repeats 15 times>, "\t", 'A' <repeats 15 times>, "\t", 'A'
<repeats 145 times>...) at codesighs.c:213
213	    int len = strlen(inString);
(gdb)
215	    while(len)
(gdb)
217	        len--;
(gdb)
219	        if(isspace(*(inString + len)))
(gdb)
221	            *(inString + len) = '\0';
(gdb)
215	    while(len)
(gdb)
217	        len--;
(gdb)
219	        if(isspace(*(inString + len)))
(gdb)
228	}
(gdb)
codesighs (inOptions=0xbffff350) at codesighs.c:261
261	        scanRes = sscanf(lineBuffer,
(gdb) i r
eax            0x0	0
ecx            0xb7fe468c	-1208072564
edx            0x82	130
ebx            0x9d8ff4	10326004
esp            0xbfffd040	0xbfffd040
ebp            0xbffff328	0xbffff328
esi            0x0	0
edi            0x0	0
eip            0x8048945	0x8048945 <codesighs+142>
eflags         0x246	[ PF ZF IF ]
cs             0x73	115
ss             0x7b	123
ds             0x7b	123
es             0x7b	123
fs             0x0	0
gs             0x33	51
(gdb) s
270	        if(6 == scanRes)
(gdb) i r
eax            0x6	6
ecx            0x414141	4276545
edx            0x0	0
ebx            0x9d8ff4	10326004
esp            0xbfffd040	0xbfffd040
ebp            0xbffff328	0xbffff328
esi            0x0	0
edi            0x0	0
eip            0x804899d	0x804899d <codesighs+230>
eflags         0x282	[ SF IF ]
cs             0x73	115
ss             0x7b	123
ds             0x7b	123
es             0x7b	123
fs             0x0	0
gs             0x33	51
(gdb)

<test file>

1	CODE	AAAAAAAAAAAAAAA	AAAAAAAAAAAAAAA	AAAAAAAAAAAAAAA	AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

</test file>

maptsvdifftool may share the same bug.
I'm not sure what all the gdb output is trying to say, but I'm guessing you're saying the problem is that we're passing buffers that aren't big enough for your test input to sscanf.

However, this is a debugging tool used in specific situations, to read as input output generated by other programs.  So even if it crashes in response to some other input, I don't see how that's a vulnerability.  This tool is not designed to handle arbitrary input, nor does it need to be.
Group: core-security
Component: General → Build Config
QA Contact: general → build-config
Yes, as I step through the program and show the registers before and after codesighs calls sscanf(), memory can be corrupted by simply feeding codesighs a file to parse.

I came across Codesighs when checking out Google's Gadgets for Linux. It was included as third party software. And I'd bet GG is not the only package that includes Codesighs.

You could also point out Firefox vulnerabilities only affect people browsing the web using Mozilla Firefox.
Comparing this bug to a bug in Firefox is ridiculous - millions of people use Firefox to consume "arbitrary content" (the Web) all the time. codesighs is far less popular, has a much more specific and narrow use case, and a victim would need to go through a lot of trouble to give it arbitrary content that will trigger this problem.
Would you like to tell me exactly which phrase, fragment or sentence of mine compares Codesighs to Firefox or anything else?

Google's Chromium operating system and Google Gadgets for Linux both include Codesighs as third party software in their packages.

Is giving someone a file to be checked by Codesighs a lot of trouble? I think not.

The fact remains that this is a vulnerability in Mozilla software and should be fixed. "ridiculous" is the lack of character Mozilla is showing by not taking it seriously.
I'm not arguing that it shouldn't be fixed. I was trying to point out that the last paragraph of comment 2 is inflammatory and not constructive. I now regret having commented at all, though.
After one day this was unmarked as a security issue. *sigh*
What command line did you use to invoke codesighs to observe the problem?  And does the white space in the file matter?  There's a line break in what's attached above, but I suspect it got mangled.  Could you attach the original?
Attached patch possible patchSplinter Review
I think this ought to fix it.

I couldn't observe any valgrind warnings or crashes running './codesighs -i your-input-file', though, even without the patch.
Attachment #417367 - Flags: review?(benjamin)
Attached file input file
In order to observe problems, I needed to convert the spaces to tabs and make the strings longer.  This does fix observed problems on this test file.
Jeremy, it's only a vulnerability if you run the tool on untrusted content, and given the nature of the tool that's very unlikely.
Attachment #417367 - Flags: review?(benjamin) → review+
We could apply that MITIGATION for any software.

"Thunderbird is only vulnerable if I open an untrusted email"

"Adobe Reader is only vulnerable if I open an untrusted PDF"

So, "Codesighs is only vulnerable if you run it on untrusted content"?

I'd love to actually see that documented by someone with authority...
(In reply to comment #11)
> "Thunderbird is only vulnerable if I open an untrusted email"
> 
> "Adobe Reader is only vulnerable if I open an untrusted PDF"

Those are the normal ways of using Thunderbird and Adobe Reader.

They're not the normal way of using codesighs.
http://hg.mozilla.org/mozilla-central/rev/833f5ebf4651
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Assignee: nobody → dbaron
Summary: Vulnerability in tools/codesighs/codesighs.c → Memory corruption vulnerability in tools/codesighs/codesighs.c
(In reply to comment #6)
> After one day this was unmarked as a security issue. *sigh*

No, it was un-hidden. The Mozilla project hates having hidden, secret stuff only a few people know about. It's against our nature, and seriously kinks our normal development style of letting as many people as possible pitch in where they're interested. It also increases the amount of work required because the security team must then coordinate and manage making sure the right developers have access to the bugs.

But protecting users from attack is another important value, and in the case of remotely triggerable vulnerabilities user safety wins out over our dislike of having to hide information. But when the risk is low (particularly local attacks) openness wins out. Doesn't mean we don't think it's a security issue.
Whiteboard: [sg:moderate local]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.