Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid modification of user input data #1285

Merged
merged 6 commits into from
Dec 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 41 additions & 23 deletions node/src/Transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -714,17 +714,20 @@ export class Transport
throw new TypeError('if given, appData must be an object');
}

// Clone given RTP parameters to not modify input data.
const clonedRtpParameters = utils.clone<RtpParameters>(rtpParameters);

// This may throw.
ortc.validateRtpParameters(rtpParameters);
ortc.validateRtpParameters(clonedRtpParameters);

// If missing or empty encodings, add one.
if (
!rtpParameters.encodings ||
!Array.isArray(rtpParameters.encodings) ||
rtpParameters.encodings.length === 0
!clonedRtpParameters.encodings ||
!Array.isArray(clonedRtpParameters.encodings) ||
clonedRtpParameters.encodings.length === 0
)
{
rtpParameters.encodings = [ {} ];
clonedRtpParameters.encodings = [ {} ];
}

// Don't do this in PipeTransports since there we must keep CNAME value in
Expand All @@ -733,9 +736,13 @@ export class Transport
{
// If CNAME is given and we don't have yet a CNAME for Producers in this
// Transport, take it.
if (!this.#cnameForProducers && rtpParameters.rtcp && rtpParameters.rtcp.cname)
if (
!this.#cnameForProducers &&
clonedRtpParameters.rtcp &&
clonedRtpParameters.rtcp.cname
)
{
this.#cnameForProducers = rtpParameters.rtcp.cname;
this.#cnameForProducers = clonedRtpParameters.rtcp.cname;
}
// Otherwise if we don't have yet a CNAME for Producers and the RTP
// parameters do not include CNAME, create a random one.
Expand All @@ -745,26 +752,26 @@ export class Transport
}

// Override Producer's CNAME.
rtpParameters.rtcp = rtpParameters.rtcp || {};
rtpParameters.rtcp.cname = this.#cnameForProducers;
clonedRtpParameters.rtcp = clonedRtpParameters.rtcp ?? {};
clonedRtpParameters.rtcp.cname = this.#cnameForProducers;
}

const routerRtpCapabilities = this.#getRouterRtpCapabilities();

// This may throw.
const rtpMapping = ortc.getProducerRtpParametersMapping(
rtpParameters, routerRtpCapabilities);
clonedRtpParameters, routerRtpCapabilities);

// This may throw.
const consumableRtpParameters = ortc.getConsumableRtpParameters(
kind, rtpParameters, routerRtpCapabilities, rtpMapping);
kind, clonedRtpParameters, routerRtpCapabilities, rtpMapping);

const producerId = id || utils.generateUUIDv4();
const requestOffset = createProduceRequest({
builder : this.channel.bufferBuilder,
builder : this.channel.bufferBuilder,
producerId,
kind,
rtpParameters,
rtpParameters : clonedRtpParameters,
rtpMapping,
keyFrameRequestDelay,
paused
Expand All @@ -787,8 +794,8 @@ export class Transport
const data =
{
kind,
rtpParameters,
type : producerTypeFromFbs(status.type),
rtpParameters : clonedRtpParameters,
type : producerTypeFromFbs(status.type),
consumableRtpParameters
};

Expand All @@ -800,7 +807,7 @@ export class Transport
producerId
},
data,
channel : this.channel,
channel : this.channel,
appData,
paused
});
Expand Down Expand Up @@ -854,8 +861,11 @@ export class Transport
throw new TypeError('if given, mid must be non empty string');
}

// Clone given RTP capabilities to not modify input data.
const clonedRtpCapabilities = utils.clone<RtpCapabilities>(rtpCapabilities);

// This may throw.
ortc.validateRtpCapabilities(rtpCapabilities!);
ortc.validateRtpCapabilities(clonedRtpCapabilities);

const producer = this.getProducerById(producerId);

Expand All @@ -874,7 +884,7 @@ export class Transport
const rtpParameters = ortc.getConsumerRtpParameters(
{
consumableRtpParameters : producer.consumableRtpParameters,
remoteRtpCapabilities : rtpCapabilities!,
remoteRtpCapabilities : clonedRtpCapabilities,
pipe,
enableRtx
}
Expand Down Expand Up @@ -996,13 +1006,17 @@ export class Transport

let type: DataProducerType;

// Clone given SCTP stream parameters to not modify input data.
let clonedSctpStreamParameters
= utils.clone<SctpStreamParameters | undefined>(sctpStreamParameters);

// If this is not a DirectTransport, sctpStreamParameters are required.
if (this.constructor.name !== 'DirectTransport')
{
type = 'sctp';

// This may throw.
ortc.validateSctpStreamParameters(sctpStreamParameters!);
ortc.validateSctpStreamParameters(clonedSctpStreamParameters!);
}
// If this is a DirectTransport, sctpStreamParameters must not be given.
else
Expand All @@ -1013,15 +1027,17 @@ export class Transport
{
logger.warn(
'produceData() | sctpStreamParameters are ignored when producing data on a DirectTransport');

clonedSctpStreamParameters = undefined;
}
}

const dataProducerId = id || utils.generateUUIDv4();
const requestOffset = createProduceDataRequest({
builder : this.channel.bufferBuilder,
builder : this.channel.bufferBuilder,
dataProducerId,
type,
sctpStreamParameters,
sctpStreamParameters : clonedSctpStreamParameters,
label,
protocol,
paused
Expand Down Expand Up @@ -1117,8 +1133,10 @@ export class Transport
if (this.constructor.name !== 'DirectTransport')
{
type = 'sctp';
sctpStreamParameters =
(utils.clone(dataProducer.sctpStreamParameters) ?? {}) as SctpStreamParameters;

sctpStreamParameters = utils.clone<SctpStreamParameters | undefined>(
dataProducer.sctpStreamParameters
) ?? {} as SctpStreamParameters;

// Override if given.
if (ordered !== undefined)
Expand Down
21 changes: 13 additions & 8 deletions node/src/Worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import * as ortc from './ortc';
import { Channel } from './Channel';
import { Router, RouterOptions } from './Router';
import { WebRtcServer, WebRtcServerOptions } from './WebRtcServer';
import { RtpCodecCapability } from './RtpParameters';
import { AppData } from './types';
import { generateUUIDv4, parseVector } from './utils';
import * as utils from './utils';
import { Event } from './fbs/notification';
import * as FbsRequest from './fbs/request';
import * as FbsWorker from './fbs/worker';
Expand Down Expand Up @@ -703,7 +704,7 @@ export class Worker<WorkerAppData extends AppData = AppData>
);
}

const webRtcServerId = generateUUIDv4();
const webRtcServerId = utils.generateUUIDv4();

const createWebRtcServerRequestOffset = new FbsWorker.CreateWebRtcServerRequestT(
webRtcServerId, fbsListenInfos
Expand Down Expand Up @@ -747,10 +748,14 @@ export class Worker<WorkerAppData extends AppData = AppData>
throw new TypeError('if given, appData must be an object');
}

// Clone given media codecs to not modify input data.
const clonedMediaCodecs =
utils.clone<RtpCodecCapability[] | undefined>(mediaCodecs);

// This may throw.
const rtpCapabilities = ortc.generateRouterRtpCapabilities(mediaCodecs);
const rtpCapabilities = ortc.generateRouterRtpCapabilities(clonedMediaCodecs);

const routerId = generateUUIDv4();
const routerId = utils.generateUUIDv4();

// Get flatbuffer builder.
const createRouterRequestOffset =
Expand Down Expand Up @@ -822,12 +827,12 @@ export function parseWorkerDumpResponse(
{
const dump: WorkerDump = {
pid : binary.pid()!,
webRtcServerIds : parseVector(binary, 'webRtcServerIds'),
routerIds : parseVector(binary, 'routerIds'),
webRtcServerIds : utils.parseVector(binary, 'webRtcServerIds'),
routerIds : utils.parseVector(binary, 'routerIds'),
channelMessageHandlers :
{
channelRequestHandlers : parseVector(binary.channelMessageHandlers()!, 'channelRequestHandlers'),
channelNotificationHandlers : parseVector(binary.channelMessageHandlers()!, 'channelNotificationHandlers')
channelRequestHandlers : utils.parseVector(binary.channelMessageHandlers()!, 'channelRequestHandlers'),
channelNotificationHandlers : utils.parseVector(binary.channelMessageHandlers()!, 'channelNotificationHandlers')
}
};

Expand Down
2 changes: 1 addition & 1 deletion node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,5 +95,5 @@ export async function createWorker<WorkerAppData extends types.AppData = types.A
*/
export function getSupportedRtpCapabilities(): RtpCapabilities
{
return utils.clone(supportedRtpCapabilities) as RtpCapabilities;
return utils.clone<RtpCapabilities>(supportedRtpCapabilities);
}
Loading
Loading