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

Status: RESOLVED DUPLICATE of bug 980800

(Testing Graveyard :: Mozmill, defect)

Description

[Georg Koppen]

Attachment: mozilla-central-asan-stacktrace

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.

Comment 1

[Georg Koppen]

FWIW this is https://bugs.torproject.org/17567 on our side.

Comment 2

[Matt Wobensmith [:mwobensmith][:matt:]]

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.

Comment 3

[Dave Huseby [:huseby]]

I will take a look soon.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 6

[Georg Koppen]

Attachment: 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

Comment 7

[Georg Koppen]

Reopening. Please adjust the product/component as needed (dunno what to enter there).

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 9

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

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.

Comment 10

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

(s/enum/union/.  Too much Rust.)

Comment 11

[Georg Koppen]

(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?

Comment 12

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

…which would mean that this is a duplicate of bug 980800 after all, if I understand correctly.

Comment 13

[Georg Koppen]

(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.

Comment 14

[Georg Koppen]

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.