From d097a29418b505ab8e744e4cdf33685bc6ac5e91 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Tue, 16 Jun 2020 11:50:43 +0300 Subject: [PATCH 1/4] Better validation --- .../src/components/labels-editor/common.ts | 53 +++++++++++++++++++ .../components/labels-editor/raw-viewer.tsx | 18 +++++-- 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/cvat-ui/src/components/labels-editor/common.ts b/cvat-ui/src/components/labels-editor/common.ts index 5ea21f53c5e1..9627be41e3cb 100644 --- a/cvat-ui/src/components/labels-editor/common.ts +++ b/cvat-ui/src/components/labels-editor/common.ts @@ -18,6 +18,59 @@ export interface Label { let id = 0; +function validateParsedAttribute(attr: Attribute): void { + if (typeof (attr.name) !== 'string') { + throw new Error(`Type of attribute name must be a string. Got value ${attr.name}`); + } + + if (!['number', 'undefined'].includes(typeof (attr.id))) { + throw new Error(`Attribute: "${attr.name}". ` + + `Type of attribute id must be a number or undefined. Got value ${attr.id}`); + } + + if (!['checkbox', 'number', 'text', 'radio', 'select'].includes((attr.input_type || '').toLowerCase())) { + throw new Error(`Attribute: "${attr.name}". ` + + `Unknown input type: ${attr.input_type}`); + } + + if (typeof (attr.mutable) !== 'boolean') { + throw new Error(`Attribute: "${attr.name}". ` + + `Mutable flag must be a boolean value. Got value ${attr.mutable}`); + } + + if (!Array.isArray(attr.values)) { + throw new Error(`Attribute: "${attr.name}". ` + + `Attribute values must be an array. Got type ${typeof (attr.values)}`); + } + + for (const value of attr.values) { + if (typeof (value) !== 'string') { + throw new Error(`Attribute: "${attr.name}". ` + + `Each value must be a string. Got value ${value}`); + } + } +} + +export function validateParsedLabel(label: Label): void { + if (typeof (label.name) !== 'string') { + throw new Error(`Type of label name must be a string. Got value ${label.name}`); + } + + if (!['number', 'undefined'].includes(typeof (label.id))) { + throw new Error(`Label "${label.name}". ` + + `Type of label id must be only a number or undefined. Got value ${label.id}`); + } + + if (!Array.isArray(label.attributes)) { + throw new Error(`Label "${label.name}". ` + + `attributes must be an array. Got type ${typeof (label.attributes)}`); + } + + for (const attr of label.attributes) { + validateParsedAttribute(attr); + } +} + export function idGenerator(): number { return --id; } diff --git a/cvat-ui/src/components/labels-editor/raw-viewer.tsx b/cvat-ui/src/components/labels-editor/raw-viewer.tsx index a19fef874796..8aadce3d2728 100644 --- a/cvat-ui/src/components/labels-editor/raw-viewer.tsx +++ b/cvat-ui/src/components/labels-editor/raw-viewer.tsx @@ -9,10 +9,7 @@ import Button from 'antd/lib/button'; import Tooltip from 'antd/lib/tooltip'; import Form, { FormComponentProps } from 'antd/lib/form/Form'; -import { - Label, - Attribute, -} from './common'; +import { Label, Attribute, validateParsedLabel } from './common'; type Props = FormComponentProps & { labels: Label[]; @@ -22,7 +19,18 @@ type Props = FormComponentProps & { class RawViewer extends React.PureComponent { private validateLabels = (_: any, value: string, callback: any): void => { try { - JSON.parse(value); + const parsed = JSON.parse(value); + if (!Array.isArray(parsed)) { + callback('Field is expected to be a JSON array'); + } + + for (const label of parsed) { + try { + validateParsedLabel(label); + } catch (error) { + callback(error.toString()); + } + } } catch (error) { callback(error.toString()); } From 4dd403c0e0399c863f22de0e4dc837385acabe0f Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Tue, 16 Jun 2020 12:04:10 +0300 Subject: [PATCH 2/4] Temporary ID generation for created labels and attributes (fixes React warning) --- .../src/components/labels-editor/raw-viewer.tsx | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/cvat-ui/src/components/labels-editor/raw-viewer.tsx b/cvat-ui/src/components/labels-editor/raw-viewer.tsx index 8aadce3d2728..40dd695c9a86 100644 --- a/cvat-ui/src/components/labels-editor/raw-viewer.tsx +++ b/cvat-ui/src/components/labels-editor/raw-viewer.tsx @@ -9,7 +9,12 @@ import Button from 'antd/lib/button'; import Tooltip from 'antd/lib/tooltip'; import Form, { FormComponentProps } from 'antd/lib/form/Form'; -import { Label, Attribute, validateParsedLabel } from './common'; +import { + Label, + Attribute, + validateParsedLabel, + idGenerator, +} from './common'; type Props = FormComponentProps & { labels: Label[]; @@ -47,7 +52,14 @@ class RawViewer extends React.PureComponent { e.preventDefault(); form.validateFields((error, values): void => { if (!error) { - onSubmit(JSON.parse(values.labels)); + const parsed = JSON.parse(values.labels); + for (const label of parsed) { + label.id = idGenerator(); + for (const attr of label.attributes) { + attr.id = idGenerator(); + } + } + onSubmit(parsed); } }); }; From b442677b801d74daa0250433ff69dc794b97214e Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Tue, 16 Jun 2020 12:11:10 +0300 Subject: [PATCH 3/4] Updated version and changelog --- CHANGELOG.md | 3 +++ cvat-ui/package-lock.json | 2 +- cvat-ui/package.json | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index deccc8e63a55..b82b13894e97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Shortcut to change color of an activated shape in new UI (Enter) () - Shortcut to switch split mode () - Built-in search for labels when create an object or change a label () +- Better validation of labels and attributes in raw viewer () ### Changed - Removed information about e-mail from the basic user information () @@ -32,6 +33,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Wrong rexex for account name validation () - Wrong description on register view for the username field () - Wrong resolution for resizing a shape () +- React warning because of not unique keys in labels viewer () + ### Security - SQL injection in Django `CVE-2020-9402` () diff --git a/cvat-ui/package-lock.json b/cvat-ui/package-lock.json index d429a17f89d9..d7d6dc4ddb92 100644 --- a/cvat-ui/package-lock.json +++ b/cvat-ui/package-lock.json @@ -1,6 +1,6 @@ { "name": "cvat-ui", - "version": "1.3.0", + "version": "1.3.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/cvat-ui/package.json b/cvat-ui/package.json index 2a9f9705c084..e15f89230fee 100644 --- a/cvat-ui/package.json +++ b/cvat-ui/package.json @@ -1,6 +1,6 @@ { "name": "cvat-ui", - "version": "1.3.0", + "version": "1.3.1", "description": "CVAT single-page application", "main": "src/index.tsx", "scripts": { From 71e400ccc16284225b4583a5bf12d42d0b81c9d5 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Tue, 16 Jun 2020 12:21:16 +0300 Subject: [PATCH 4/4] Fixed bug with existing labels --- cvat-ui/src/components/labels-editor/raw-viewer.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cvat-ui/src/components/labels-editor/raw-viewer.tsx b/cvat-ui/src/components/labels-editor/raw-viewer.tsx index 40dd695c9a86..532046f42bbf 100644 --- a/cvat-ui/src/components/labels-editor/raw-viewer.tsx +++ b/cvat-ui/src/components/labels-editor/raw-viewer.tsx @@ -54,9 +54,9 @@ class RawViewer extends React.PureComponent { if (!error) { const parsed = JSON.parse(values.labels); for (const label of parsed) { - label.id = idGenerator(); + label.id = label.id || idGenerator(); for (const attr of label.attributes) { - attr.id = idGenerator(); + attr.id = attr.id || idGenerator(); } } onSubmit(parsed);