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

fix: make sha256() throw on comptime strings of length > 128 #907

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Collisions in getter method ids are now handled and reported properly: PR [#875](https://github.com/tact-lang/tact/pull/875)
- The `sha256()` function now throws on comptime strings of length > 128: PR [#907](https://github.com/tact-lang/tact/pull/907)

### Release contributors

Expand Down
4 changes: 2 additions & 2 deletions docs/src/content/docs/book/operators.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ Unary here means that they are applied only to one operand of the given expressi

Unary operators can be one of the two types:

* prefix — placed before the expression.
* postfix (or suffix) — placed after the expression.
* Prefix — placed before the expression.
* Postfix (or suffix) — placed after the expression.

### Non-null assert, `!!` {#unary-non-null-assert}

Expand Down
10 changes: 7 additions & 3 deletions docs/src/content/docs/ref/core-math.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,15 @@ fun sha256(data: Slice): Int;
fun sha256(data: String): Int;
```

Computes and returns the [SHA-256][sha-2] hash as an $256$-bit unsigned [`Int{:tact}`][int] from a passed [`Slice{:tact}`][slice] or [`String{:tact}`][p] `data`.
Computes and returns the [SHA-256][sha-2] hash as a $256$-bit unsigned [`Int{:tact}`][int] from a passed [`Slice{:tact}`][slice] or [`String{:tact}`][p] `data`.

In case `data` is a [`String{:tact}`][p] it should have a number of bits divisible by $8$, and in case it's a [`Slice{:tact}`][slice] it must **also** have no references (i.e. only up to 1023 bits of data in total).
In case `data` is a [`String{:tact}`][p] it should have a number of bits divisible by $8$, and in case it's a [`Slice{:tact}`][slice] it must **also** have no references (i.e. only up to $1023$ bits of data in total). This function tries to resolve constant string values in [compile-time](/ref/core-comptime) whenever possible.

This function tries to resolve constant string values in [compile-time](/ref/core-comptime) whenever possible.
:::caution

If there are more than $128$ bytes in the provided [`String{:tact}`][p] or [`Slice{:tact}`][slice], a compilation error is raised if the compiler can track it, and if not — the hashes of such long arguments would be the same on TON, which might cause collisions. Prefer to hash smaller arguments to prevent such cases.

:::

Usage examples:

Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/ref/stdlib-dns.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Source code (FunC): [dns.fc#L1](https://github.com/tact-lang/tact/blob/e69c7fc99
native dnsInternalNormalize(src: Slice): Slice;
```

Normalizes the internal DNS representation of the [`Slice{:tact}`][slice]. The passed [`Slice{:tact}`][slice] must not have any references, otherwise an exception wit [exit code 134](/book/exit-codes#134) will be thrown: `Invalid argument`.
Normalizes the internal DNS representation of the [`Slice{:tact}`][slice]. The passed [`Slice{:tact}`][slice] must not have any references, otherwise an exception with [exit code 134](/book/exit-codes#134) will be thrown: `Invalid argument`.

Source code (FunC): [dns.fc#L125](https://github.com/tact-lang/tact/blob/e69c7fc99dc9be3fa5ff984456c03ffe8fed3677/stdlib/libs/dns.fc#L125)

Expand Down
26 changes: 20 additions & 6 deletions src/abi/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
writeExpression,
writeValue,
} from "../generator/writers/writeExpression";
import { throwCompilationError } from "../errors";
import { TactConstEvalError, throwCompilationError } from "../errors";
import { evalConstantExpression } from "../constEval";
import { getErrorId } from "../types/resolveErrors";
import { AbiFunction } from "./AbiFunction";
Expand Down Expand Up @@ -368,23 +368,37 @@ export const GlobalFunctions: Map<string, AbiFunction> = new Map([

// String case
if (arg0.name === "String") {
let str: string | undefined;

// Try const-eval
try {
const str = evalConstantExpression(
str = evalConstantExpression(
resolved[0]!,
ctx.ctx,
) as string;
if (Buffer.from(str).length > 128) {
} catch (error) {
if (
!(error instanceof TactConstEvalError) ||
error.fatal
)
throw error;
}

// If const-eval did succeed
if (str !== undefined) {
const dataSize = Buffer.from(str).length;
if (dataSize > 128) {
throwCompilationError(
"sha256 expects string argument with byte length <= 128",
`data is too large for sha256 hash, expected up to 128 bytes, got ${dataSize}`,
ref,
);
}
return BigInt(
"0x" + sha256_sync(str).toString("hex"),
).toString(10);
} catch (e) {
// Not a constant
}

// Otherwise, revert back to runtime hash through SHA256U
const exp = writeExpression(resolved[0]!, ctx);
return `string_hash(${exp})`;
}
Expand Down
14 changes: 9 additions & 5 deletions src/interpreter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Address, beginCell, BitString, Cell, toNano } from "@ton/core";
import { Address, beginCell, BitString, Cell, Slice, toNano } from "@ton/core";
import { paddedBufferToBits } from "@ton/core/dist/boc/utils/paddedBits";
import * as crc32 from "crc-32";
import { evalConstantExpression } from "./constEval";
Expand Down Expand Up @@ -1038,10 +1038,14 @@ export class Interpreter {
}
case "sha256": {
ensureFunArity(1, ast.args, ast.loc);
const str = ensureString(
this.interpretExpression(ast.args[0]!),
ast.args[0]!.loc,
);
const expr = this.interpretExpression(ast.args[0]!);
if (expr instanceof Slice) {
throwNonFatalErrorConstEval(
"slice argument is currently not supported",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this returns a non fatal error, it means that if someone actually passes a Slice as argument to sha256, the interpreter will not evaluate the expression, but the code generation will still accept the expression. For example, this:

let a = sha256(s);   // where s has type Slice       

will be accepted by the compiler, but will not be simplified by the interpreter.

Is this the expected behavior? Is it possible to add a positive test where sha256 receives a Slice?

ast.loc,
);
}
const str = ensureString(expr, ast.args[0]!.loc);
const dataSize = Buffer.from(str).length;
if (dataSize > 128) {
throwErrorConstEval(
Expand Down
13 changes: 13 additions & 0 deletions src/test/compilation-failed/abi-global-errors.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { __DANGER_resetNodeId } from "../../grammar/ast";
import { itShouldNotCompile } from "./util";

describe("abi/global.ts errors", () => {
beforeEach(() => {
__DANGER_resetNodeId();
});

itShouldNotCompile({
testName: "sha256-expects-string-or-slice",
errorMessage: "sha256 expects string or slice argument",
});
});
5 changes: 5 additions & 0 deletions src/test/compilation-failed/const-eval-failed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,9 @@ describe("fail-const-eval", () => {
errorMessage:
"Cannot evaluate expression to a constant: ascii string cannot be empty",
});
itShouldNotCompile({
testName: "const-eval-sha256-too-large",
errorMessage:
"Cannot evaluate expression to a constant: data is too large for sha256 hash, expected up to 128 bytes, got 129",
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
contract Sha256 {
receive() {
sha256("------------------------------------------------------------------------------------------------------------------------------129");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
contract Sha256 {
val: Int = 0;
receive() {
sha256(self.val);
}
}
10 changes: 10 additions & 0 deletions src/test/compilation-failed/tact.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,16 @@
"name": "scope-const-shadows-stdlib-ident",
"path": "./contracts/scope-const-shadows-stdlib-ident.tact",
"output": "./contracts/output"
},
{
"name": "sha256-expects-string-or-slice",
"path": "./contracts/sha256-expects-string-or-slice.tact",
"output": "./contracts/output"
},
{
"name": "const-eval-sha256-too-large",
"path": "./contracts/const-eval-sha256-too-large.tact",
"output": "./contracts/output"
}
]
}
Loading