heap-buffer-overflow when using ctypes to call into NSS

RESOLVED DUPLICATE of bug 980800

Status

defect
--
critical
RESOLVED DUPLICATE of bug 980800
4 years ago
3 years ago

People

(Reporter: gk, Unassigned)

Tracking

(4 keywords)

Firefox Tracking Flags

(firefox45 affected)

Details

()

Attachments

(2 attachments)

Reporter

Description

4 years ago
When trying to run Mozmill tests the Address Sanitizer finds a heap-buffer-overflow (see attached stack trace).

Nicolas found this while trying to get our hardened builds (based on ESR 38) running with our test suite.

Steps to reproduce:

1) Grab the latest ASan build for Linux which Mozilla provides and extract it somewhere.
2) Make sure you have a Mozmill setup available
3) Set ASAN_SYMBOLIZER_PATH to the proper path
4) Try running a test like

mozmill -b /path/to/firefox/firefox -t /path/to/mozmill-tests/firefox/tests/functional/testPreferences/testRestoreHomepageToDefault.js

Results: Crash + attached stack trace
Expected Results: Test is failing/passing.
Reporter

Comment 1

4 years ago
FWIW this is https://bugs.torproject.org/17567 on our side.
Dave, would you mind taking a look? We're trying to determine if this is a Tor bug, a Fx ESR bug or something in our Mozmill test suite.
Flags: needinfo?(huseby)
I will take a look soon.
Flags: needinfo?(huseby)
FYI Mozmill is about to be abandoned. We already stopped actively supporting this framework, but only have to run keep running it for Firefox releases before 38ESR. It's replacement is Marionette.

I wonder if that bug is somehow a dupe of bug 980800. The crash looks to be at the same location.
Group: core-security
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Component: Security → Mozmill
Product: Core → Testing
QA Contact: hskupin
Resolution: --- → DUPLICATE
Duplicate of bug: 980800
Reporter

Comment 6

4 years ago
Posted file crashtest.xpi
This is not a Mozmill issue. Attached is a PoC .xpi which show the same problem.
Steps to reproduce:

1) Take the latest ASan nightly for 64bit Linux
2) Create the integer preference `extensions.crashtest.port` and add a port for the server socket
3) Set `xpinstall.signatures.required` to `false`
4) Install the attached .xpi and restart

Result: heap-buffer-overflow
Reporter

Comment 7

4 years ago
Reopening. Please adjust the product/component as needed (dunno what to enter there).
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Reporter

Updated

4 years ago
Summary: heap-buffer-overflow when running Mozmill tests → heap-buffer-overflow when using ctypes to call into NSS
Great info Georg! That explains why I haven't found anything obvious in Mozmill also because it happened that rarely. Looks like we now have a solid testcase available.

Not sure if the core-security flag still make sense but setting it for now. Also moving into NSS which seems to be the affected component here.
Assignee: nobody → nobody
Severity: normal → critical
Component: Mozmill → Libraries
Product: Testing → NSS
QA Contact: hskupin
Version: Trunk → trunk
Group: crypto-core-security
Group: crypto-core-security
Keywords: sec-other
I reproduced this under GDB (tip: set ASAN_OPTIONS=abort_on_error=1):

(gdb) call DumpJSStack()
0 Sockets.ServerSocket(aPort = 8888) ["resource://crashtest/modules/Sockets.jsm":115]
    this = [object Object]

    112   let opt = NSS.Types.PRSocketOptionData();
    113   opt.non_blocking = NSS.Sockets.PR_TRUE;
    114   opt.option = NSS.Sockets.PR_SockOpt_Nonblocking;
    115   status = NSS.Sockets.PR_SetSocketOption(fd, opt.address());

Okay, so what is a PRSocketOptionData?

  PRSocketOptionData: ctypes.StructType("PRSocketOptionData", [
    {'option' : ctypes.int32_t},
    {'non_blocking': ctypes.int32_t}
  ]),

But what is a PRSocketOptionData really?

(gdb) p sizeof(PRSocketOptionData)
$3 = 232

Well that's a bad sign.  Checking the source:

  typedef struct PRSocketOptionData
  {
      PRSockOption option;
      union
      {
          PRUintn ip_ttl;             /* IP time to live */

Looking good so far…

          PRSize max_segment;         /* Maximum segment size */

…but this is where the problems start.  PRSize on amd64 is 8 bytes and 8-byte-aligned, so the entire value enum is at offset 8, not offset 4 as in the ctypes struct.  gdb agrees:

(gdb) p (size_t)&((PRSocketOptionData*)0)->value.non_blocking
$10 = 8

So this is a bug in the JS code.
(s/enum/union/.  Too much Rust.)
Reporter

Comment 11

4 years ago
(In reply to Jed Davis [:jld] from comment #9)
> So this is a bug in the JS code.

This basically means this is no bug in NSS, right? If so, could you please adjust the product/component accordingly in order to have the right folks noticing this issue?
Flags: needinfo?(jld)
Assignee: nobody → nobody
Component: Libraries → Mozmill
Flags: needinfo?(jld)
Product: NSS → Testing
QA Contact: hskupin
Version: trunk → unspecified
…which would mean that this is a duplicate of bug 980800 after all, if I understand correctly.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 980800
Reporter

Comment 13

4 years ago
(In reply to Jed Davis [:jld] from comment #12)
> …which would mean that this is a duplicate of bug 980800 after all, if I
> understand correctly.

No. The attached .xpi PoC has nothing to do with Mozmill (see comment:6 as well) and I get the same heap-buffer-overflow. So, reopening again. I am still not exactly sure which components this belongs to, though.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Reporter

Comment 14

4 years ago
Ah, you meant with "bug in the JS code" a bug in Mozmill/the PoC. Let's keep this closed then and sorry for the noise.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 980800
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.