From 2411db6b3663bf04712c84ad4341992885590a12 Mon Sep 17 00:00:00 2001 From: Zaid Abdulla Date: Fri, 28 Aug 2020 00:49:05 -0700 Subject: [PATCH] Better handling of invalid CPE messages --- lib/cwmp.ts | 23 +++++++-- lib/session.ts | 133 +++++++++++++++++++++++-------------------------- lib/soap.ts | 115 +++++++++++++++++++++++++++++------------- lib/types.ts | 8 +-- 4 files changed, 166 insertions(+), 113 deletions(-) diff --git a/lib/cwmp.ts b/lib/cwmp.ts index 03d4568..0e9584b 100644 --- a/lib/cwmp.ts +++ b/lib/cwmp.ts @@ -1376,8 +1376,14 @@ async function listenerAsync( httpResponse.setHeader("Content-Length", Buffer.byteLength(msg)); httpResponse.writeHead(400, { Connection: "close" }); if (sessionContext) { - if (sessionContext.debug) + if (sessionContext.debug) { + debug.incomingHttpRequest( + httpRequest, + sessionContext.deviceId, + body.toString() + ); debug.outgoingHttpResponse(httpResponse, sessionContext.deviceId, msg); + } } else { const cacheSnapshot = await localCache.getCurrentSnapshot(); const d = !!localCache.getConfig( @@ -1393,7 +1399,10 @@ async function listenerAsync( return e; } ); - if (d) debug.outgoingHttpResponse(httpResponse, null, msg); + if (d) { + debug.incomingHttpRequest(httpRequest, null, body.toString()); + debug.outgoingHttpResponse(httpResponse, null, msg); + } } httpResponse.end(msg); return; @@ -1420,6 +1429,11 @@ async function listenerAsync( httpResponse.writeHead(400, { Connection: "close" }); if (sessionContext) { if (sessionContext.debug) { + debug.incomingHttpRequest( + httpRequest, + sessionContext.deviceId, + bodyStr + ); debug.outgoingHttpResponse( httpResponse, sessionContext.deviceId, @@ -1441,7 +1455,10 @@ async function listenerAsync( return e; } ); - if (d) debug.outgoingHttpResponse(httpResponse, null, error.message); + if (d) { + debug.incomingHttpRequest(httpRequest, null, bodyStr); + debug.outgoingHttpResponse(httpResponse, null, error.message); + } } httpResponse.end(error.message); return; diff --git a/lib/session.ts b/lib/session.ts index 26822ae..ddbad1e 100644 --- a/lib/session.ts +++ b/lib/session.ts @@ -192,7 +192,7 @@ export async function inform( ]; for (const p of rpcReq.parameterList) { - const path = Path.parse(p[0]); + const path = p[0]; params.push([ path, timestamp, @@ -2436,12 +2436,12 @@ export async function rpcResponse( const res = rpcRes as GetParameterValuesResponse; const parameterList: [string, string | number | boolean, string][] = []; for (const p of res.parameterList) { - if (p[1] !== req.instanceValues[p[0]]) { + if (p[1] !== req.instanceValues[p[0].toString()]) { const v = device.sanitizeParameterValue([ - req.instanceValues[p[0]], + req.instanceValues[p[0].toString()], p[2] as string, ]); - parameterList.push([p[0], v[0], v[1]]); + parameterList.push([p[0].toString(), v[0], v[1]]); } } @@ -2481,11 +2481,7 @@ export async function rpcResponse( sessionContext.deviceData.timestamps.revision = revision; sessionContext.deviceData.attributes.revision = revision; - let toClear, - root: Path, - missing, - params: [Path, number, Attributes?][], - wildcardPath: Path; + let toClear: Clear[]; if (rpcRes.name === "GetParameterValuesResponse") { if (rpcReq.name !== "GetParameterValues") @@ -2494,7 +2490,7 @@ export async function rpcResponse( for (const p of rpcRes.parameterList) { toClear = device.set( sessionContext.deviceData, - Path.parse(p[0]), + p[0], timestamp, { object: [timestamp, 0], @@ -2510,7 +2506,7 @@ export async function rpcResponse( for (const p of rpcRes.parameterList) { toClear = device.set( sessionContext.deviceData, - Path.parse(p[0]), + p[0], timestamp, { notification: [timestamp, p[1] as number], @@ -2523,67 +2519,41 @@ export async function rpcResponse( if (rpcReq.name !== "GetParameterNames") throw new Error("Response name does not match request name"); + let root: Path; if (rpcReq.parameterPath.endsWith(".")) root = Path.parse(rpcReq.parameterPath.slice(0, -1)); else root = Path.parse(rpcReq.parameterPath); - wildcardPath = Path.parse("*"); - params = [[root.concat(wildcardPath), timestamp]]; - - // Some clients don't report all ancestors explicitly - missing = {}; - - for (const p of rpcRes.parameterList) { - let i = p[0].length - 1; - while ((i = p[0].lastIndexOf(".", i - 1)) > rpcReq.parameterPath.length) - missing[p[0].slice(0, i)] |= 0; - - if (p[0].endsWith(".")) { - missing[p[0].slice(0, -1)] |= 1; - const path = Path.parse(p[0].slice(0, -1)); - if (!rpcReq.nextLevel) - params.push([path.concat(wildcardPath), timestamp]); - - params.push([ - path, - timestamp, - { object: [timestamp, 1], writable: [timestamp, p[1] ? 1 : 0] }, - ]); - } else { - missing[p[0]] |= 1; - params.push([ - Path.parse(p[0]), - timestamp, - { object: [timestamp, 0], writable: [timestamp, p[1] ? 1 : 0] }, - ]); + // Sort to help fill in missing params + rpcRes.parameterList.sort((a, b) => { + const pa = a[0]; + const pb = b[0]; + const l = Math.min(pa.length, pb.length); + for (let i = 0; i < l; ++i) { + if (pa.segments[i] > pb.segments[i]) return 1; + if (pa.segments[i] < pb.segments[i]) return -1; } - } - - for (const [k, v] of Object.entries(missing)) { - if (v === 0) { - // TODO consider showing a warning - const path = Path.parse(k); - params.push([ - path, - timestamp, - { object: [timestamp, 1], writable: [timestamp, 0] }, - ]); - params.push([path.concat(wildcardPath), timestamp]); - } - } - - // Sort such that: - // - Longer params come first in order to work around client issue - // where object paths can have no trailing dot. - // - Parameters come before wildcard paths. - params.sort((a, b) => { - let al = a[0].length; - let bl = b[0].length; - if (b[0].segments[bl - 1] === "*") bl *= -1; - if (a[0].segments[al - 1] === "*") al *= -1; - return bl - al; + return pa.length - pb.length; }); + // Fill in missing unreported parent params + for (let idx = 1; idx < rpcRes.parameterList.length; ++idx) { + const prev = rpcRes.parameterList[idx - 1][0]; + const cur = rpcRes.parameterList[idx][0]; + let offset = 0; + for (let i = cur.length - 2; i >= 0; --i) { + if (i < prev.length && prev.segments[i] === cur.segments[i]) { + // Set object to true in case CPE didn't indicate that it's an object + if (i === prev.length - 1) rpcRes.parameterList[idx - 1][1] = true; + break; + } + // TODO consider showing a warning + rpcRes.parameterList.splice(idx, 0, [cur.slice(0, i + 1), true, true]); + ++offset; + } + idx += offset; + } + if (!root.length) { for (const n of [ "DeviceID", @@ -2600,12 +2570,31 @@ export async function rpcResponse( } } - for (const p of params) { + const wildcardPath = Path.parse("*"); + const wildcardParams: Path[] = [root.concat(wildcardPath)]; + + for (const [path, object, writable] of rpcRes.parameterList) { + if (object && !rpcReq.nextLevel) + wildcardParams.push(path.concat(wildcardPath)); + toClear = device.set( sessionContext.deviceData, - p[0], - p[1], - p[2], + path, + timestamp, + { + object: [timestamp, object ? 1 : 0], + writable: [timestamp, writable ? 1 : 0], + }, + toClear + ); + } + + for (const path of wildcardParams) { + toClear = device.set( + sessionContext.deviceData, + path, + timestamp, + null, toClear ); } @@ -2870,13 +2859,13 @@ export async function rpcFault( export async function deserialize( sessionContextString: string ): Promise { - const sessionContext = JSON.parse(sessionContextString); + const sessionContext = JSON.parse(sessionContextString) as SessionContext; for (const decs of sessionContext.declarations) - for (const d of decs) d.path = Path.parse(d.path); + for (const d of decs) d.path = Path.parse((d.path as unknown) as string); const deviceData = initDeviceData(); - for (const r of sessionContext.deviceData) { + for (const r of (sessionContext.deviceData as unknown) as any[]) { const path = deviceData.paths.add(Path.parse(r[0])); if (r[1]) deviceData.trackers.set(path, r[1]); diff --git a/lib/soap.ts b/lib/soap.ts index dc8c522..b22c627 100644 --- a/lib/soap.ts +++ b/lib/soap.ts @@ -49,6 +49,7 @@ import { RequestDownloadRequest, AcsResponse, } from "./types"; +import Path from "./common/path"; const SERVER_NAME = `GenieACS/${VERSION}`; @@ -94,26 +95,22 @@ let warnings: Record[]; const memoizedParseAttrs = memoize(parseAttrs); -function parseBool(v): boolean { - v = "" + v; - if (v === "true" || v === "TRUE" || v === "True" || v === "1") return true; - else if (v === "false" || v === "FALSE" || v === "False" || v === "0") - return false; - else return null; +function parseBool(v: string): boolean { + if (v === "true" || v === "1") return true; + if (v === "false" || v === "0") return false; + return null; } function event(xml: Element): string[] { return xml.children .filter((n) => n.localName === "EventStruct") - .map((c) => - c.children.find((n) => n.localName === "EventCode").text.trim() - ); + .map((c) => c.children.find((n) => n.localName === "EventCode").text); } -function parameterInfoList(xml: Element): [string, boolean][] { +function parameterInfoList(xml: Element): [Path, boolean, boolean][] { return xml.children .filter((e) => e.localName === "ParameterInfoStruct") - .map<[string, boolean]>((e) => { + .map<[Path, boolean, boolean]>((e) => { let param: string, value: string; for (const c of e.children) { switch (c.localName) { @@ -136,22 +133,25 @@ function parameterInfoList(xml: Element): [string, boolean][] { parsed = false; } - return [param, parsed]; + if (param && !param.endsWith(".")) + return [Path.parse(param), false, parsed]; + else return [Path.parse(param.slice(0, -1)), true, parsed]; }); } -const getValueType = memoize((str: string) => - parseAttrs(str) - .find((s) => s.localName === "type") - .value.trim() -); +const getValueType = memoize((str: string) => { + const attrs = parseAttrs(str); + for (const attr of attrs) if (attr.localName === "type") return attr.value; + + return null; +}); function parameterValueList( xml: Element -): [string, string | number | boolean, string][] { +): [Path, string | number | boolean, string][] { return xml.children .filter((e) => e.localName === "ParameterValueStruct") - .map<[string, string | number | boolean, string]>((e) => { + .map<[Path, string | number | boolean, string]>((e) => { let valueElement: Element, param: string; for (const c of e.children) { switch (c.localName) { @@ -164,7 +164,14 @@ function parameterValueList( } } - const valueType = getValueType(valueElement.attrs); + let valueType = getValueType(valueElement.attrs); + if (!valueType) { + warnings.push({ + message: "Invalid value type attribute", + parameter: param, + }); + valueType = "xsd:string"; + } const value = decodeEntities(valueElement.text); let parsed: string | number | boolean = value; @@ -197,14 +204,14 @@ function parameterValueList( } } - return [param, parsed, valueType]; + return [Path.parse(param), parsed, valueType]; }); } -function parameterAttributeList(xml: Element): [string, number, string[]][] { +function parameterAttributeList(xml: Element): [Path, number, string[]][] { return xml.children .filter((e) => e.localName === "ParameterAttributeStruct") - .map<[string, number, string[]]>((e) => { + .map<[Path, number, string[]]>((e) => { let notificationElement: Element, accessListElement: Element, param: string; @@ -222,21 +229,20 @@ function parameterAttributeList(xml: Element): [string, number, string[]][] { } } - const notification = decodeEntities(notificationElement.text); - const notif = +notification; - - if (notif !== parseInt(notification)) { + let notification = parseInt(notificationElement.text); + if (isNaN(notification)) { warnings.push({ message: "Invalid notification attribute", parameter: param, }); + notification = 0; } const accessList = accessListElement.children .filter((c) => c.localName === "string") .map((c) => decodeEntities(c.text)); - return [param, notif, accessList]; + return [Path.parse(param), notification, accessList]; }); } @@ -314,9 +320,24 @@ function SetParameterValues(methodRequest): string { } function SetParameterValuesResponse(xml: Element): SetParameterValuesResponse { + let status: number; + + for (const c of xml.children) { + switch (c.localName) { + case "Status": + status = parseInt(c.text); + break; + } + } + + if (!(status >= 0)) { + warnings.push({ message: "Invalid SetParameterValuesResponse status" }); + status = 0; + } + return { name: "SetParameterValuesResponse", - status: parseInt(xml.children.find((n) => n.localName === "Status").text), + status: status, }; } @@ -359,11 +380,11 @@ function AddObject(methodRequest): string { } function AddObjectResponse(xml: Element): AddObjectResponse { - let instanceNumber, status; + let instanceNumber: string, status: number; for (const c of xml.children) { switch (c.localName) { case "InstanceNumber": - instanceNumber = parseInt(c.text); + instanceNumber = c.text; break; case "Status": status = parseInt(c.text); @@ -371,6 +392,11 @@ function AddObjectResponse(xml: Element): AddObjectResponse { } } + if (!(status >= 0)) { + warnings.push({ message: "Invalid AddObjectResponse status" }); + status = 0; + } + return { name: "AddObjectResponse", instanceNumber: instanceNumber, @@ -387,9 +413,24 @@ function DeleteObject(methodRequest): string { } function DeleteObjectResponse(xml: Element): DeleteObjectResponse { + let status: number; + + for (const c of xml.children) { + switch (c.localName) { + case "Status": + status = parseInt(c.text); + break; + } + } + + if (!(status >= 0)) { + warnings.push({ message: "Invalid DeleteObjectResponse status" }); + status = 0; + } + return { name: "DeleteObjectResponse", - status: parseInt(xml.children.find((n) => n.localName === "Status").text), + status: status, }; } @@ -438,7 +479,7 @@ function Download(methodRequest): string { } function DownloadResponse(xml: Element): DownloadResponse { - let status, startTime, completeTime; + let status: number, startTime: number, completeTime: number; for (const c of xml.children) { switch (c.localName) { case "Status": @@ -453,6 +494,11 @@ function DownloadResponse(xml: Element): DownloadResponse { } } + if (!(status >= 0)) { + warnings.push({ message: "Invalid DownloadResponse status" }); + status = 0; + } + return { name: "DownloadResponse", status: status, @@ -462,7 +508,8 @@ function DownloadResponse(xml: Element): DownloadResponse { } function Inform(xml: Element): InformRequest { - let retryCount, evnt, parameterList; + let retryCount: number, evnt: string[]; + let parameterList: [Path, string | number | boolean, string][]; const deviceId = { Manufacturer: null, OUI: null, diff --git a/lib/types.ts b/lib/types.ts index 2369ff2..ecec48e 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -308,17 +308,17 @@ export type CpeResponse = export interface GetParameterNamesResponse { name: "GetParameterNamesResponse"; - parameterList: [string, boolean][]; + parameterList: [Path, boolean, boolean][]; } export interface GetParameterValuesResponse { name: "GetParameterValuesResponse"; - parameterList: [string, string | number | boolean, string][]; + parameterList: [Path, string | number | boolean, string][]; } export interface GetParameterAttributesResponse { name: "GetParameterAttributesResponse"; - parameterList: [string, number, string[]][]; + parameterList: [Path, number, string[]][]; } export interface SetParameterValuesResponse { @@ -372,7 +372,7 @@ export interface InformRequest { }; event: string[]; retryCount: number; - parameterList: [string, string | number | boolean, string][]; + parameterList: [Path, string | number | boolean, string][]; } export interface TransferCompleteRequest {