Commit Graph

30 Commits

Author SHA1 Message Date
Damien Arrachequesne
9d39f1f080 fix(parser): add a limit to the number of binary attachments
Backported from main: b25738c416

When a packet contains binary elements, the built-in parser does not modify them and simply sends them in their own WebSocket frame.

Example: `socket.emit("some event", Buffer.of(1,2,3))`

is encoded and transferred as:

- 1st frame: 51-["some event",{"_placeholder":true,"num":0}]
- 2nd frame: <buffer 01 02 03>

where:

- `5` is the type of the packet (binary message)
- `1` is the number of binary attachments
- `-` is the separator
- `["some event",{"_placeholder":true,"num":0}]` is the payload (including the placeholder)

On the receiving end, the parser reads the number of attachments and buffers them until they are all received.

Before this change, the built-in parser accepted any number of binary attachments, which could be exploited to make the server run out of memory.

The number of attachments is now limited to 10, which should be sufficient for most use cases.

The limit can be increased with a custom `parser`:

```js
import { Encoder, Decoder } from "socket.io-parser";

const io = new Server({
  parser: {
    Encoder,
    Decoder: class extends Decoder {
      constructor() {
        super({
          maxAttachments: 20
        });
      }
    }
  }
});
```
2026-03-17 16:00:15 +01:00
Arnau Fugarolas Barbena
ee00660749 fix: check the format of the event name (#125)
A packet like '2[{"toString":"foo"}]' was decoded as:

{
  type: EVENT,
  data: [ { "toString": "foo" } ]
}

Which would then throw an error when passed to the EventEmitter class:

> TypeError: Cannot convert object to primitive value
>    at Socket.emit (node:events:507:25)
>    at .../node_modules/socket.io/lib/socket.js:531:14

Backported from 3b78117bf6
2024-07-22 11:05:42 +02:00
Damien Arrachequesne
fb21e422fc fix: check the format of the index of each attachment
A specially crafted packet could be incorrectly decoded.

Example:

```js
const decoder = new Decoder();

decoder.on("decoded", (packet) => {
  console.log(packet.data); // prints [ 'hello', [Function: splice] ]
})

decoder.add('51-["hello",{"_placeholder":true,"num":"splice"}]');
decoder.add(Buffer.from("world"));
```

As usual, please remember not to trust user input.

Backported from b5d0cb7dc5
2022-11-09 11:21:34 +01:00
bcaller
89197a05c4 fix: prevent DoS (OOM) via massive packets (#95)
When maxHttpBufferSize is large (1e8 bytes), a payload of length 100MB
can be sent like so:

99999991:422222222222222222222222222222222222222222222...

This massive packet can cause OOM via building up many many
`ConsOneByteString` objects due to concatenation:
99999989 `ConsOneByteString`s and then converting the massive integer to
a `Number`.

The performance can be improved to avoid this by using `substring`
rather than building the string via concatenation.

Below I tried one payload of length 7e7 as the 1e8 payload took so
long to process that it timed out before running out of memory.

```
==== JS stack trace =========================================

    0: ExitFrame [pc: 0x13c5b79]
Security context: 0x152fe7b808d1 <JSObject>
    1: decodeString [0x2dd385fb5d1] [/node_modules/socket.io-parser/index.js:~276] [pc=0xf59746881be](this=0x175d34c42b69 <JSGlobal Object>,0x14eccff10fe1 <Very long string[69999990]>)
    2: add [0x31fc2693da29] [/node_modules/socket.io-parser/index.js:242] [bytecode=0xa7ed6554889 offset=11](this=0x0a2881be5069 <Decoder map = 0x3ceaa8bf48c9>,0x14eccff10fe1 <Very...

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
 1: 0xa09830 node::Abort() [node]
 2: 0xa09c55 node::OnFatalError(char const*, char const*) [node]
 3: 0xb7d71e v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [node]
 4: 0xb7da99 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [node]
 5: 0xd2a1f5  [node]
 6: 0xd2a886 v8::internal::Heap::RecomputeLimits(v8::internal::GarbageCollector) [node]
 7: 0xd37105 v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [node]
 8: 0xd37fb5 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [node]
 9: 0xd3965f v8::internal::Heap::HandleGCRequest() [node]
10: 0xce8395 v8::internal::StackGuard::HandleInterrupts() [node]
11: 0x1042cb6 v8::internal::Runtime_StackGuard(int, unsigned long*, v8::internal::Isolate*) [node]
12: 0x13c5b79  [node]
```

Backported from master: dcb942d24d
2021-01-09 14:43:12 +01:00
JinHyuk Kim
48f340ec12 [refactor] Fix a small typo and code styling (#88) 2018-11-07 22:53:25 +01:00
Damien Arrachequesne
92c530da47 [fix] Properly handle JSON.stringify errors (#84)
JSON.stringify method throws when passed a circular object.
2018-02-28 22:07:33 +01:00
Damien Arrachequesne
dc4f475a45 [revert] Move binary detection to the parser
So that we can skip the binary check.
2018-02-28 21:55:26 +01:00
Damien Arrachequesne
f0a7df1059 [fix] Ensure packet data is an array (#83)
Related: https://github.com/socketio/socket.io/pull/3140
2018-02-25 09:05:16 +01:00
Damien Arrachequesne
f44256c523 [feat] Move binary detection to the parser (#66) 2017-04-24 23:20:51 +02:00
Jimmy Karl Roland Wärting
e39f5a8c6a [chore] Use native JSON and drop support for older nodejs versions (#64) 2017-04-03 23:15:24 +02:00
Gatsbill
2314c10f4f [perf] Small optimisations (#57) 2016-12-30 22:25:49 +01:00
Damien Arrachequesne
c2d0a08d7f [refactor] Remove useless variable (#55) 2016-12-17 03:34:00 +01:00
Gatsbill
97721957b9 [refactor] Use strict equality when possible (#52) 2016-12-17 02:32:21 +01:00
Tom Atkinson
9b479bcee6 [perf] Split try catch into separate function (#40) 2016-10-21 01:07:30 +02:00
Damien Arrachequesne
e8f7c32a43 [chore] Remove deprecated isarray dependency (#46)
Closes #38
2016-10-21 01:03:33 +02:00
Naoyuki Kanezawa
f2a825557c Merge pull request #23 from chylli/fix-event-order
fix the order of events
2015-11-23 00:33:00 +09:00
Guillermo Rauch
59d2329731 index: fix off-by-one bound checks 2015-03-03 10:22:09 -08:00
Guillermo Rauch
e77fc4c32b index: fix potential infinite loop with malicious binary packet 2015-02-03 16:26:53 -08:00
Chylli
e8f660f77f fix the order of events 2014-12-26 16:30:52 +08:00
Guillermo Rauch
0ae9a4fba6 prevent direct Buffer reference that breaks browserify 2014-09-04 10:14:23 +02:00
Reid Burke
035fa7dded Upgrade component-emitter to 1.1.2
Switch from depending on a tarball URL to the published
component-emitter package at its latest version.

Change all references to emitter module to the new
component-emitter name.
2014-06-04 15:03:37 -07:00
Kevin Roark
5ca1da1c7e bump protocol version here too 2014-05-31 23:30:39 -07:00
Kevin Roark
ca4f42a922 added a BINARY_ACK type 2014-05-30 18:41:47 -07:00
Kevin Roark
ed7cd9410c updated protocol version 2014-03-24 14:55:35 -04:00
Kevin Roark
c8cce3e8c0 removed the 'in' slowness 2014-03-06 17:24:58 -05:00
Kevin Roark
2f729e07b4 Encoding ack packets as binary
So that arbitrary data can be passed back and forth in ack callbacks.
2014-03-06 16:52:08 -05:00
Kevin Roark
47df0694f5 Fixed the object check in binary.removeBlobs
Remove blobs has to iterate over a javascript object and asynchronously
remove the blobs / files. It does this by iterating over arrays and
objects in the larger object recursively.

Problem was in checking for object to iterate over, wasn't checking if
that object was binary data itself. So it was working, but really slowly,
by iterating over every byte in a Buffer and checking it for blobs.
Much faster now :)
2014-03-04 23:50:36 -05:00
Kevin Roark
5bea0bf41c Protocol is now class-based
Separated the encoding and decoding into two public-facing objects,
Encoder and Decoder.

Both objects take nothing on construction. Encoder has a single method,
encode, that mimics the previous version's function encode (takes a
packet object and a callback). Decoder has a single method too, add, that
takes any object (packet string or binary data). Decoder emits a 'decoded'
event when it has received all of the parts of a packet. The only
parameter for the decoded event is the reconstructed packet.

I am hesitant about the Encoder.encode vs Decoder.add thing. Should it be
more consistent, or should it stay like this where the function names are
more descriptive?

Also, rewrote the test helper functions to deal with new event-based
decoding. Wrote a new test in test/arraybuffer.js that tests for memory
leaks in Decoder as well.
2014-02-27 17:46:20 -05:00
Kevin Roark
299849b002 A faster and smaller binary parser and protocol
This is a squash of a few commits. Below is a small summary of commits.

Results from it: before the build size of socket.io-client was ~250K.
Now it is ~215K.
Tests I was doing here
(https://github.com/kevin-roark/socketio-binaryexample/tree/speed-testing)
take about 1/4 - 1/5 as long with this commit compared to msgpack.

The first was the initial rewrite of the encoding, which removes msgpack
and instead uses a sequence of engine.write's for a binary event. The
first write is the packet metadata with placeholders in the json for
any binary data. Then the following events are the raw binary data that
get filled by the placeholders.

The second commit was bug fixes that made the tests pass.

The third commit was removing unnecssary packages from package.json.

Fourth commit was adding nice comments, and 5th commit was merging
upstream.

The remaining commits involved merging with actual socket.io-parser,
rather than the protocol repository. Oops.
2014-02-26 22:31:39 -05:00
Guillermo Rauch
86725d1e92 moved from socket.io-protocol 2014-02-19 15:26:26 -08:00