chore: introduce @trezor/metadata package #16110

mroz22 · 2024-12-28T12:02:44Z

this PR introduces a new package @trezor/metadata. This package is intended to gather all metadata/labelling logic in it so that the suite-mobile team can use it easily when the time comes (cc @matejkriz).

resolve #16124

mroz22 · 2024-12-28T12:05:24Z

packages/metadata/src/services/google.ts

                if (!json?.access_token || !json?.refresh_token) {
                    throw new Error('Could not retrieve the tokens.');
                }
                Client.accessToken = json.access_token;
                Client.refreshToken = json.refresh_token;
            } catch {
+                // TODO: it looks like that this actually does not work. I tested it even on develop by throwing an artifical error somewhere above.


cc @komret. not a big deal. if I am not wrong, this doesn't work. at least it helped me discover another potential runtime issue I fixed here https://github.com/trezor/trezor-suite/pull/16110/files#diff-56d44c230c949c29acb06aed65bf1fffef52e38cf29e86546c87b0e92b2c9c25R165

mroz22 · 2024-12-28T12:06:09Z

packages/metadata/src/services/AbstractProvider.ts

+    'request-code': (url: string, callback: (credentials: Credentials) => void) => void;
+};
+
+export abstract class AbstractMetadataProvider extends TypedEmitter<Events> {


just changed it to event emitter. no changes besides that

github-actions · 2024-12-28T12:07:00Z

🚀 Expo preview is ready!

Project → trezor-suite-preview
Platforms → android, ios
Scheme → trezorsuitelite
Runtime Version → 21
More info

Learn more about 𝝠 Expo Github Action

mroz22 · 2024-12-28T12:07:05Z

packages/metadata/src/types.ts

+    tags: Record<number, PasswordTag>;
+};
+
+export type Credentials =


only Credentials type was added

packages/metadata/src/api.ts

+                    return Promise.resolve({ success: true as const });
+                }
+
+                delete nextMetadata.outputLabels[payload.txid][payload.outputIndex];


To fix the problem, we need to ensure that payload.txid cannot be used to modify Object.prototype. This can be achieved by validating the payload.txid before using it as a key in the nextMetadata.outputLabels object. We will reject any keys that are __proto__, constructor, or prototype.

packages/metadata/src/api.ts

+                    nextMetadata.outputLabels[payload.txid] = {};
+                }
+
+                nextMetadata.outputLabels[payload.txid][payload.outputIndex] = payload.value;


To fix the prototype pollution issue, we need to ensure that the keys used in object assignments cannot be __proto__, constructor, or prototype. This can be achieved by adding a check before the assignment to reject these keys. Alternatively, we can use a Map object instead of a plain object for nextMetadata.outputLabels.

The best way to fix the problem without changing existing functionality is to add a check before the assignment to ensure the key is not one of the dangerous properties.

packages/metadata/src/api.ts

+
+        if (payload.type === 'accountLabel') {
+            if (typeof payload.value !== 'string' || payload.value.length === 0) {
+                delete nextMetadata.accountLabel;


To fix the problem, we need to ensure that the keys used to modify the nextMetadata object cannot include dangerous values like __proto__. One effective way to achieve this is by using a Map object instead of a plain JavaScript object for nextMetadata. Map objects do not have prototype properties and are therefore immune to prototype pollution.

We will replace the plain object nextMetadata with a Map and update the code to use Map methods for setting and deleting values. This change will be made in the addAccountMetadata method.

mroz22 commented Dec 28, 2024

View reviewed changes

mroz22 force-pushed the metadata-package branch 5 times, most recently from c6ee3b1 to 935948a Compare January 3, 2025 12:17

mroz22 force-pushed the metadata-package branch from 4b3cb1f to 7c6725b Compare January 14, 2025 09:09

mroz22 added 2 commits January 15, 2025 13:09

chore: introduce @trezor/metadata package

0afba98

brutal wip: move maximum logic to metadata package

6958513

mroz22 force-pushed the metadata-package branch from 7c6725b to 6958513 Compare January 15, 2025 15:30

github-advanced-security bot found potential problems Jan 15, 2025

View reviewed changes

@@ -442,2 +442,5 @@
                     if (payload.type === 'outputLabel') {
+                        if (typeof payload.txid === 'string' && (payload.txid === '__proto__' || payload.txid === 'constructor' || payload.txid === 'prototype')) {
+                            return Promise.resolve({ success: false as const, error: 'Invalid txid' });
+                        }
                         if (typeof payload.value !== 'string' || payload.value.length === 0) {

@@ -456,2 +456,5 @@
                         } else {
+                            if (payload.txid === '__proto__' || payload.txid === 'constructor' || payload.txid === 'prototype') {
+                                return Promise.resolve({ success: false as const, error: 'Invalid key' });
+                            }
                             if (!nextMetadata.outputLabels[payload.txid]) {

@@ -439,3 +439,3 @@
-                    const nextMetadata = cloneObject(data ?? DEFAULT_ACCOUNT_METADATA) as AccountLabels;
+                    const nextMetadata = new Map(Object.entries(cloneObject(data ?? DEFAULT_ACCOUNT_METADATA) as AccountLabels));
@@ -443,3 +443,3 @@
                         if (typeof payload.value !== 'string' || payload.value.length === 0) {
-                            if (!nextMetadata.outputLabels[payload.txid]) {
+                            if (!nextMetadata.get('outputLabels').get(payload.txid)) {
                                 // If we try to delete already deleted label it's ok.
@@ -451,12 +451,12 @@
-                            delete nextMetadata.outputLabels[payload.txid][payload.outputIndex];
-                            if (Object.keys(nextMetadata.outputLabels[payload.txid]).length === 0) {
-                                delete nextMetadata.outputLabels[payload.txid];
+                            nextMetadata.get('outputLabels').get(payload.txid).delete(payload.outputIndex);
+                            if (nextMetadata.get('outputLabels').get(payload.txid).size === 0) {
+                                nextMetadata.get('outputLabels').delete(payload.txid);
                             }
                         } else {
-                            if (!nextMetadata.outputLabels[payload.txid]) {
-                                nextMetadata.outputLabels[payload.txid] = {};
+                            if (!nextMetadata.get('outputLabels').get(payload.txid)) {
+                                nextMetadata.get('outputLabels').set(payload.txid, new Map());
                             }
-                            nextMetadata.outputLabels[payload.txid][payload.outputIndex] = payload.value;
+                            nextMetadata.get('outputLabels').get(payload.txid).set(payload.outputIndex, payload.value);
                         }
@@ -466,5 +466,5 @@
                         if (typeof payload.value !== 'string' || payload.value.length === 0) {
-                            delete nextMetadata.addressLabels[payload.defaultValue];
+                            nextMetadata.get('addressLabels').delete(payload.defaultValue);
                         } else {
-                            nextMetadata.addressLabels[payload.defaultValue] = payload.value;
+                            nextMetadata.get('addressLabels').set(payload.defaultValue, payload.value);
                         }
@@ -474,5 +474,5 @@
                         if (typeof payload.value !== 'string' || payload.value.length === 0) {
-                            delete nextMetadata.accountLabel;
+                            nextMetadata.delete('accountLabel');
                         } else {
-                            nextMetadata.accountLabel = payload.value;
+                            nextMetadata.set('accountLabel', payload.value);
                         }
@@ -489,5 +489,5 @@
                         data: {
-                            accountLabel: nextMetadata.accountLabel,
-                            outputLabels: nextMetadata.outputLabels,
-                            addressLabels: nextMetadata.addressLabels,
+                            accountLabel: nextMetadata.get('accountLabel'),
+                            outputLabels: Object.fromEntries(nextMetadata.get('outputLabels')),
+                            addressLabels: Object.fromEntries(nextMetadata.get('addressLabels')),
                         },

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: introduce @trezor/metadata package #16110

chore: introduce @trezor/metadata package #16110

mroz22 commented Dec 28, 2024 •

edited

Loading

mroz22 Dec 28, 2024

mroz22 Dec 28, 2024

github-actions bot commented Dec 28, 2024 •

edited

Loading

mroz22 Dec 28, 2024

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

chore: introduce @trezor/metadata package #16110

Are you sure you want to change the base?

chore: introduce @trezor/metadata package #16110

Conversation

mroz22 commented Dec 28, 2024 • edited Loading

mroz22 Dec 28, 2024

Choose a reason for hiding this comment

mroz22 Dec 28, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 28, 2024 • edited Loading

mroz22 Dec 28, 2024

Choose a reason for hiding this comment

mroz22 commented Dec 28, 2024 •

edited

Loading

github-actions bot commented Dec 28, 2024 •

edited

Loading