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

Entrypoint name encoding can violate Tezos specifications #265

Closed
fcremo opened this issue Feb 25, 2020 · 0 comments
Closed

Entrypoint name encoding can violate Tezos specifications #265

fcremo opened this issue Feb 25, 2020 · 0 comments

Comments

@fcremo
Copy link

@fcremo fcremo commented Feb 25, 2020

Hi! Doyensec has been engaged to perform a security assessment of this library. I will be opening issues to document our findings.

Description

Tezos transactions can define custom entrypoints starting from protocol version 5. According to the official documentation, custom entrypoints maximum length is 31 characters, but taquito does not enforce this limitation when encoding or decoding transactions.

This is an extract from the documentation:

The entrypoint format is as follows:
one byte 0 for entrypoint %default
one byte 1 for entrypoint %root
[...]
one byte 255 for a named entrypoint, 
then one byte of entrypoint name size (limited to 31), and the name itself

This is the relevant code from codec.ts:

export const entrypointDecoder = (value: Uint8ArrayConsumer) => {
  const preamble = pad(value.consume(1)[0], 2);

  if (preamble in entrypointMapping) {
    return entrypointMapping[preamble];
  } else {
    const entry = extractRequiredLen(value, 1);
    return Buffer.from(entry).toString('utf8');
  }
};

export const entrypointEncoder = (entrypoint: string) => {
  if (entrypoint in entrypointMappingReverse) {
    return `${entrypointMappingReverse[entrypoint]}`;
  } else {
    const value = { string: entrypoint };
    return `ff${valueEncoder(value).slice(8)}`;
  }
};

Observe that no checks are performed on the length of the entrypoint name being encoded or decoded. You might want to consider enforcing the size limit required by the specification.

<sales pitch>If you’re looking for an independent vendor to perform security testing or to develop security automation solutions, let us know! https://doyensec.com</sales pitch>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

1 participant
You can’t perform that action at this time.