Skip to content

fix: reject an out-of-range id in toUUID instead of silently truncating - #83

Open
spokodev wants to merge 1 commit into
oculus42:developfrom
spokodev:fix-out-of-range-touuid
Open

fix: reject an out-of-range id in toUUID instead of silently truncating#83
spokodev wants to merge 1 commit into
oculus42:developfrom
spokodev:fix-out-of-range-touuid

Conversation

@spokodev

Copy link
Copy Markdown

Problem

toUUID can silently return a wrong UUID. Because maxLength is ceil(128 / log2(alphabetLength)) (rounded up), a maxLength-character id can encode a value larger than 2^128 - 1. Such an id decodes to more than 32 hex characters, and restoreUUID's unanchored match keeps only the first 32 and drops the overflow:

const short = require('short-uuid');
const t = short.createTranslator(short.constants.flickrBase58); // maxLength 22

const x = 'ZZZZZZZZZZZZZZZZZZZZZZ'; // 22 chars, in-alphabet, decodes to > 2^128-1
t.validate(x);            // true
t.toUUID(x);              // '1d5b20ad-2d23-30b1-0a7b-b82b9f63ffff'  (truncated)
t.fromUUID(t.toUUID(x));  // '4CfuZZZZZZZZZZZZZZZZZZ'  !== x

This breaks the documented toUUID / fromUUID round-trip. validate(x, true) (rigorous) already returns false for these ids, so the library treats them as invalid, yet toUUID accepts and corrupts them.

Cause

src/translate.ts:

export const restoreUUID = (config, shortId) =>
  config.hexFromAlphabet(shortId)
    .padStart(32, '0')
    .match(/(\w{8})(\w{4})(\w{4})(\w{4})(\w{12})/)  // unanchored: matches first 32 hex
    ?.slice(1)
    .join('-');

When the decoded hex is longer than 32 characters the overflow is silently discarded.

Fix

Guard the decoded length and throw (matching the library's existing behaviour of throwing on invalid input, e.g. a duplicate-character alphabet), and anchor the match so the 32-hex expectation is explicit. Normal ids (<= 2^128-1) are unaffected.

Tests

Added a case asserting toUUID throws on an out-of-range id and that a normally generated id still round-trips. It fails on develop (the id silently truncates) and passes with the fix. Full suite green (195 tests).

`maxLength` is `ceil(128 / log2(alphabetLength))`, which rounds up, so a
maxLength-character id can encode a value larger than 2^128-1. Such an id
decodes to more than 32 hex characters, and the unanchored match in
restoreUUID silently kept the first 32 and dropped the overflow, returning
a wrong UUID:

    const t = short.createTranslator(short.constants.flickrBase58);
    const x = 'ZZZZZZZZZZZZZZZZZZZZZZ';   // 22 chars, decodes to > 2^128-1
    t.validate(x);             // true
    t.fromUUID(t.toUUID(x));   // !== x  (silently corrupted)

Guard the decoded hex length and throw, matching the library's existing
behaviour of throwing on invalid input. Anchor the match so the 32-hex
expectation is explicit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant