Skip to content

Commit

Permalink
fix: free strings after being read to prevent potential memory leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
Im-Beast committed Sep 9, 2024
1 parent 7275503 commit f6d4d14
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 19 deletions.
6 changes: 3 additions & 3 deletions impls/monero.ts/checksum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export class ChecksumError extends Error {
* @returns {ChecksumError} which contains information about why checksum failed
*/
export async function validateChecksum(): Promise<ChecksumError | null> {
const cppHeaderHash = readCString(await dylib.symbols.MONERO_checksum_wallet2_api_c_h());
const cppHeaderHash = await readCString(await dylib.symbols.MONERO_checksum_wallet2_api_c_h());
const tsHeaderHash = moneroChecksum.wallet2_api_c_h_sha256;

const errors: string[] = [];
Expand All @@ -30,14 +30,14 @@ export async function validateChecksum(): Promise<ChecksumError | null> {
errorCode++;
}

const cppSourceHash = readCString(await dylib.symbols.MONERO_checksum_wallet2_api_c_cpp());
const cppSourceHash = await readCString(await dylib.symbols.MONERO_checksum_wallet2_api_c_cpp());
const tsSourceHash = moneroChecksum.wallet2_api_c_cpp_sha256;
if (cppSourceHash !== tsSourceHash) {
errors.push(`ERR: CPP source file check mismatch ${cppSourceHash} == ${tsSourceHash}`);
errorCode++;
}

const cppExportHash = readCString(await dylib.symbols.MONERO_checksum_wallet2_api_c_exp());
const cppExportHash = await readCString(await dylib.symbols.MONERO_checksum_wallet2_api_c_exp());
const tsExportHash = moneroChecksum.wallet2_api_c_exp_sha256;
if (cppExportHash !== tsExportHash) {
if (Deno.build.os !== "darwin") {
Expand Down
8 changes: 8 additions & 0 deletions impls/monero.ts/src/bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,14 @@ export const symbols = {
result: "pointer",
},
//#endregion

"MONERO_free": {
nonblocking: true,
// void* ptr
parameters: ["pointer"],
// void
result: "void",
},
} as const;

type MoneroTsDylib = Deno.DynamicLibrary<typeof symbols>;
Expand Down
6 changes: 3 additions & 3 deletions impls/monero.ts/src/pending_transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class PendingTransaction {
const error = await dylib.symbols.MONERO_PendingTransaction_errorString(this.#pendingTxPtr);
if (!error) return null;

return readCString(error) || null;
return await readCString(error) || null;
}

async throwIfError(sanitize = true): Promise<void> {
Expand Down Expand Up @@ -50,7 +50,7 @@ export class PendingTransaction {
);
if (!result) return null;
await this.throwIfError();
return readCString(result) || null;
return await readCString(result) || null;
}

async amount(): Promise<bigint> {
Expand All @@ -72,7 +72,7 @@ export class PendingTransaction {
);
if (!result) return null;
await this.throwIfError(sanitize);
return readCString(result) || null;
return await readCString(result) || null;
}

async txCount(): Promise<bigint> {
Expand Down
12 changes: 6 additions & 6 deletions impls/monero.ts/src/transaction_info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ export class TransactionInfo {

async description(): Promise<string> {
const description = await dylib.symbols.MONERO_TransactionInfo_description(this.#txInfoPtr);
return readCString(description) || "";
return await readCString(description) || "";
}

async subaddrIndex(): Promise<string> {
const subaddrIndex = await dylib.symbols.MONERO_TransactionInfo_subaddrIndex(this.#txInfoPtr);
return readCString(subaddrIndex) || "";
return await readCString(subaddrIndex) || "";
}

async subaddrAccount(): Promise<number> {
Expand All @@ -64,7 +64,7 @@ export class TransactionInfo {

async label(): Promise<string> {
const label = await dylib.symbols.MONERO_TransactionInfo_label(this.#txInfoPtr);
return readCString(label) || "";
return await readCString(label) || "";
}

async confirmations(): Promise<bigint> {
Expand All @@ -77,7 +77,7 @@ export class TransactionInfo {

async hash(): Promise<string> {
const hash = await dylib.symbols.MONERO_TransactionInfo_hash(this.#txInfoPtr);
return readCString(hash) || "";
return await readCString(hash) || "";
}

async timestamp(): Promise<bigint> {
Expand All @@ -86,7 +86,7 @@ export class TransactionInfo {

async paymentId(): Promise<string> {
const paymentId = await dylib.symbols.MONERO_TransactionInfo_paymentId(this.#txInfoPtr);
return readCString(paymentId) || "";
return await readCString(paymentId) || "";
}

async transfersCount(): Promise<number> {
Expand All @@ -99,6 +99,6 @@ export class TransactionInfo {

async transfersAddress(index: number): Promise<string> {
const transfersAddress = await dylib.symbols.MONERO_TransactionInfo_transfers_address(this.#txInfoPtr, index);
return readCString(transfersAddress) || "";
return await readCString(transfersAddress) || "";
}
}
15 changes: 11 additions & 4 deletions impls/monero.ts/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
import { dylib } from "../mod.ts";

export type Sanitizer = () => void | PromiseLike<void>;

const textEncoder = new TextEncoder();
export function CString(string: string): Deno.PointerValue<string> {
return Deno.UnsafePointer.of(textEncoder.encode(`${string}\x00`));
}

export function readCString(pointer: Deno.PointerObject): string;
export function readCString(pointer: Deno.PointerValue): string | null;
export function readCString(pointer: Deno.PointerValue): string | null {
// This method reads string from the given pointer and frees the string
// SAFETY: Do not use readCString twice on the same pointer as it will cause double free
// In that case just use `Deno.UnsafePointerView(ptr).getCString()` directly
export async function readCString(pointer: Deno.PointerObject): Promise<string>;
export async function readCString(pointer: Deno.PointerValue): Promise<string | null>;
export async function readCString(pointer: Deno.PointerValue): Promise<string | null> {
if (!pointer) return null;
return new Deno.UnsafePointerView(pointer).getCString();
const string = new Deno.UnsafePointerView(pointer).getCString();
await dylib.symbols.MONERO_free(pointer);
return string;
}
6 changes: 3 additions & 3 deletions impls/monero.ts/src/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export class Wallet {
const error = await this.errorString();
throw new Error(`Failed getting address from a wallet: ${error ?? "<Error unknown>"}`);
}
return readCString(address);
return await readCString(address);
}

async balance(accountIndex = 0): Promise<bigint> {
Expand All @@ -174,7 +174,7 @@ export class Wallet {
const error = await dylib.symbols.MONERO_Wallet_errorString(this.#walletPtr);
if (!error) return null;

return readCString(error) || null;
return await readCString(error) || null;
}

async throwIfError(sanitize = true): Promise<void> {
Expand Down Expand Up @@ -253,7 +253,7 @@ export class Wallet {
const error = await this.errorString();
throw new Error(`Failed getting subaddress label from a wallet: ${error ?? "<Error unknown>"}`);
}
return readCString(label);
return await readCString(label);
}

async setSubaddressLabel(accountIndex: number, addressIndex: number, label: string): Promise<void> {
Expand Down

0 comments on commit f6d4d14

Please sign in to comment.