736589f87

Denys Vlasenko

2025-07-06

SOURCE BUG Duration: 74s

Subject

libbb/yescrypt: reduce the number of function parameters

SOURCE BUG

This commit was genuinely broken at the time (not related to our build system): libbb/yescrypt eklenirken (2025-07-06 - 2025-07-07 arasi) once unutulan kaynak dosyalari (pw_encrypt_yes.c, hash_sha256_block.c, pw_ascii64.c), ardindan 64-bit'te derlenmeyen bir 'buflen' degiskeni (alg-yescrypt-kdf.c) vardi - busybox'in kendi kodundaki gercek hatalar (e5d3a8763 ile duzeltildi)

→ Bu hatanin tam hikayesini ve duzeltme diffini gor

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

libbb/yescrypt/alg-yescrypt-common.c +32 -42 libbb/yescrypt/alg-yescrypt-kdf.c +51 -53 libbb/yescrypt/alg-yescrypt-platform.c +15 -22 libbb/yescrypt/alg-yescrypt.h +38 -49

libbb/yescrypt/alg-yescrypt-common.c+32 -42

@@ -179,62 +179,64 @@ fail:
	}

	uint8_t *yescrypt_r(
-	yescrypt_local_t *local,
	const uint8_t *passwd, size_t passwdlen,
	const uint8_t *setting,
	uint8_t *buf, size_t buflen)
	{
-	unsigned char saltbin[64], hashbin[32];
+	yescrypt_ctx_t yctx[1];
+	unsigned char hashbin32[32];
	const uint8_t src, saltstr, *saltend;
	uint8_t *dst;
-	size_t need, prefixlen, saltstrlen, saltlen;
+	size_t need, prefixlen, saltstrlen;
	uint32_t flavor, N_log2;
-	yescrypt_params_t params = { .p = 1 };
+
+	memset(yctx, 0, sizeof(yctx));
+	yctx->param.p = 1;

	/* we assume setting starts with "$y$" (caller must ensure this) */
	src = setting + 3;

	src = decode64_uint32(&flavor, src, 0);
	//if (!src)
-	// return NULL;
+	// goto fail;

	if (flavor < YESCRYPT_RW) {
-	params.flags = flavor;
+	yctx->param.flags = flavor;
	} else if (flavor <= YESCRYPT_RW + (YESCRYPT_RW_FLAVOR_MASK >> 2)) {
-	params.flags = YESCRYPT_RW + ((flavor - YESCRYPT_RW) << 2);
+	yctx->param.flags = YESCRYPT_RW + ((flavor - YESCRYPT_RW) << 2);
	} else {
-	return NULL;
+	goto fail;
	}

	src = decode64_uint32(&N_log2, src, 1);
	if (/!src \|\|/ N_log2 > 63)
-	return NULL;
-	params.N = (uint64_t)1 << N_log2;
+	goto fail;
+	yctx->param.N = (uint64_t)1 << N_log2;

-	src = decode64_uint32(&params.r, src, 1);
+	src = decode64_uint32(&yctx->param.r, src, 1);
	if (!src)
-	return NULL;
+	goto fail;
	if (*src != '$') {
	uint32_t have;
	src = decode64_uint32(&have, src, 1);
	if (have & 1)
-	src = decode64_uint32(&params.p, src, 2);
+	src = decode64_uint32(&yctx->param.p, src, 2);
	if (have & 2)
-	src = decode64_uint32(&params.t, src, 1);
+	src = decode64_uint32(&yctx->param.t, src, 1);
	if (have & 4)
-	src = decode64_uint32(&params.g, src, 1);
+	src = decode64_uint32(&yctx->param.g, src, 1);
	if (have & 8) {
	uint32_t NROM_log2;
	src = decode64_uint32(&NROM_log2, src, 1);
	if (/!src \|\|/ NROM_log2 > 63)
-	return NULL;
-	params.NROM = (uint64_t)1 << NROM_log2;
+	goto fail;
+	yctx->param.NROM = (uint64_t)1 << NROM_log2;
	}
	}
	if (!src)
-	return NULL;
+	goto fail;
	if (*src != '$')
-	return NULL;
+	goto fail;

	saltstr = src + 1;
	src = (uint8_t )strchrnul((char )saltstr, '$');
@@ -242,8 +244,8 @@ uint8_t *yescrypt_r(
	saltstrlen = src - saltstr; /* len("<salt>") */
	/* src points to end of salt ('$' or NUL byte), won't be used past this point */

-	saltlen = sizeof(saltbin);
-	saltend = decode64(saltbin, &saltlen, saltstr, saltstrlen);
+	yctx->saltlen = sizeof(yctx->salt);
+	saltend = decode64(yctx->salt, &yctx->saltlen, saltstr, saltstrlen);
	if (saltend != saltstr + saltstrlen)
	goto fail; /* saltbin[] is too small, or bad char during decode */

@@ -251,34 +253,22 @@ uint8_t *yescrypt_r(
	if (need > buflen \|\| need < prefixlen)
	goto fail;

-	if (yescrypt_kdf(local, passwd, passwdlen, saltbin, saltlen,
-	&params, hashbin, sizeof(hashbin)))
+	if (yescrypt_kdf32(yctx, passwd, passwdlen, hashbin32))
	goto fail;

	dst = mempcpy(buf, setting, prefixlen);
	*dst++ = '$';
-	dst = encode64(dst, buflen - (dst - buf), hashbin, sizeof(hashbin));
-	explicit_bzero(hashbin, sizeof(hashbin));
+	dst = encode64(dst, buflen - (dst - buf), hashbin32, sizeof(hashbin32));
	if (!dst \|\| dst >= buf + buflen)
-	return NULL;
+	goto fail;

	dst = 0; / NUL termination */
-
+	ret:
+	free_region(yctx->local);
+	explicit_bzero(yctx, sizeof(yctx));
+	explicit_bzero(hashbin32, sizeof(hashbin32));
	return buf;
-
	fail:
-	explicit_bzero(saltbin, sizeof(saltbin));
-	explicit_bzero(hashbin, sizeof(hashbin));
-	return NULL;
-	}
-
-	int yescrypt_init_local(yescrypt_local_t *local)
-	{
-	init_region(local);
-	return 0;
-	}
-
-	int yescrypt_free_local(yescrypt_local_t *local)
-	{
-	return free_region(local);
+	buf = NULL;
+	goto ret;
	}

libbb/yescrypt/alg-yescrypt-kdf.c+51 -53

@@ -798,29 +798,27 @@ static void smix(uint8_t *B, size_t r, uint32_t N, uint32_t p, uint32_t t,
	* This optimized implementation currently limits N to the range from 4 to
	* 2^31, but other implementations might not.
	*/
-	static int yescrypt_kdf_body(
-	yescrypt_local_t *local,
-	const uint8_t *passwd, size_t passwdlen,
-	const uint8_t *salt, size_t saltlen,
-	yescrypt_flags_t flags, uint64_t N, uint32_t r, uint32_t p, uint32_t t,
-	uint64_t NROM,
-	uint8_t *buf, size_t buflen)
+	static int yescrypt_kdf32_body(
+	yescrypt_ctx_t *yctx,
+	const uint8_t *passwd, size_t passwdlen,
+	yescrypt_flags_t flags, uint64_t N, uint32_t t,
+	uint8_t *buf32)
	{
	const salsa20_blk_t *VROM;
	size_t B_size, V_size, XY_size, need;
	uint8_t B, S;
	salsa20_blk_t V, XY;
	uint8_t sha256[32];
-	uint8_t dk[sizeof(sha256)], *dkp = buf;
+	uint8_t dk[sizeof(sha256)], *dkp = buf32;

	/* Sanity-check parameters */
	switch (flags & YESCRYPT_MODE_MASK) {
	case 0: /* classic scrypt - can't have anything non-standard */
-	if (flags \|\| t \|\| NROM)
+	if (flags \|\| t \|\| yctx->param.NROM)
	goto out_EINVAL;
	break;
	case YESCRYPT_WORM:
-	if (flags != YESCRYPT_WORM \|\| NROM)
+	if (flags != YESCRYPT_WORM \|\| yctx->param.NROM)
	goto out_EINVAL;
	break;
	case YESCRYPT_RW:
@@ -842,6 +840,9 @@ static int yescrypt_kdf_body(
	if (buflen > (((uint64_t)1 << 32) - 1) * 32)
	goto out_EINVAL;
	#endif
+	{
+	const uint32_t r = yctx->param.r;
+	const uint32_t p = yctx->param.p;
	if ((uint64_t)r * (uint64_t)p >= 1 << 30)
	goto out_EINVAL;
	if (N > UINT32_MAX)
@@ -862,7 +863,7 @@ static int yescrypt_kdf_body(
	}

	VROM = NULL;
-	if (NROM)
+	if (yctx->param.NROM)
	goto out_EINVAL;

	/* Allocate memory */
@@ -883,15 +884,14 @@ static int yescrypt_kdf_body(
	if (need < S_size)
	goto out_EINVAL;
	}
-	if (local->aligned_size < need) {
-	if (free_region(local))
-	return -1;
-	if (!alloc_region(local, need))
-	return -1;
+	if (yctx->local->aligned_size < need) {
+	free_region(yctx->local);
+	alloc_region(yctx->local, need);
+	dbg("allocated local:%u", need);
	}
	if (flags & YESCRYPT_ALLOC_ONLY)
	return -3; /* expected "failure" */
-	B = (uint8_t *)local->aligned;
+	B = (uint8_t *)yctx->local->aligned;
	V = (salsa20_blk_t )((uint8_t )B + B_size);
	XY = (salsa20_blk_t )((uint8_t )V + V_size);
	S = NULL;
@@ -906,28 +906,28 @@ static int yescrypt_kdf_body(
	passwdlen = sizeof(sha256);
	}

-	PBKDF2_SHA256(passwd, passwdlen, salt, saltlen, 1, B, B_size);
+	PBKDF2_SHA256(passwd, passwdlen, yctx->salt, yctx->saltlen, 1, B, B_size);

	if (flags)
	memcpy(sha256, B, sizeof(sha256));

	if (p == 1 \|\| (flags & YESCRYPT_RW)) {
-	smix(B, r, N, p, t, flags, V, NROM, VROM, XY, S, sha256);
+	smix(B, r, N, p, t, flags, V, yctx->param.NROM, VROM, XY, S, sha256);
	} else {
	uint32_t i;
	for (i = 0; i < p; i++) {
	smix(&B[(size_t)128 * r * i], r, N, 1, t, flags, V,
-	NROM, VROM, XY, NULL, NULL);
+	yctx->param.NROM, VROM, XY, NULL, NULL);
	}
	}

-	dkp = buf;
-	if (flags && buflen < sizeof(dk)) {
+	dkp = buf32;
+	if (flags && /buflen:/32 < sizeof(dk)) {
	PBKDF2_SHA256(passwd, passwdlen, B, B_size, 1, dk, sizeof(dk));
	dkp = dk;
	}

-	PBKDF2_SHA256(passwd, passwdlen, B, B_size, 1, buf, buflen);
+	PBKDF2_SHA256(passwd, passwdlen, B, B_size, 1, buf32, /buflen:/32);

	/*
	* Except when computing classic scrypt, allow all computation so far
@@ -941,11 +941,11 @@ static int yescrypt_kdf_body(
	HMAC_SHA256_Buf(dkp, sizeof(dk), "Client Key", 10, sha256);
	/* Compute StoredKey */
	{
-	size_t clen = buflen;
+	size_t clen = /buflen:/32;
	if (clen > sizeof(dk))
	clen = sizeof(dk);
	SHA256_Buf(sha256, sizeof(sha256), dk);
-	memcpy(buf, dk, clen);
+	memcpy(buf32, dk, clen);
	}
	}

@@ -960,6 +960,7 @@ static int yescrypt_kdf_body(
	out_EINVAL:
	errno = EINVAL;
	return -1;
+	}
	}

	/**
@@ -970,21 +971,18 @@ out_EINVAL:
	* the addition of g, which controls hash upgrades (0 for no upgrades so far).
	*/
	static
-	int yescrypt_kdf(
-	yescrypt_local_t *local,
+	int yescrypt_kdf32(
+	yescrypt_ctx_t *yctx,
	const uint8_t *passwd, size_t passwdlen,
-	const uint8_t *salt, size_t saltlen,
-	const yescrypt_params_t *params,
-	uint8_t *buf, size_t buflen)
+	uint8_t *buf32)
	{
-	yescrypt_flags_t flags = params->flags;
-	uint64_t N = params->N;
-	uint32_t r = params->r;
-	uint32_t p = params->p;
-	uint32_t t = params->t;
-	uint32_t g = params->g;
-	uint64_t NROM = params->NROM;
-	uint8_t dk[32];
+	yescrypt_flags_t flags = yctx->param.flags;
+	uint64_t N = yctx->param.N;
+	uint32_t r = yctx->param.r;
+	uint32_t p = yctx->param.p;
+	uint32_t t = yctx->param.t;
+	uint32_t g = yctx->param.g;
+	uint8_t dk32[32];
	int retval;

	/* Support for hash upgrades has been temporarily removed */
@@ -998,30 +996,30 @@ int yescrypt_kdf(
	&& N / p >= 0x100
	&& N / p * r >= 0x20000
	) {
-	if (yescrypt_kdf_body(local,
-	passwd, passwdlen, salt, saltlen,
-	flags \| YESCRYPT_ALLOC_ONLY, N, r, p, t, NROM,
-	buf, buflen) != -3
+	if (yescrypt_kdf32_body(yctx,
+	passwd, passwdlen,
+	flags \| YESCRYPT_ALLOC_ONLY, N, t,
+	buf32) != -3
	) {
	errno = EINVAL;
	return -1;
	}
-	retval = yescrypt_kdf_body(local,
-	passwd, passwdlen, salt, saltlen,
-	flags \| YESCRYPT_PREHASH, N >> 6, r, p, 0, NROM,
-	dk, sizeof(dk));
+	retval = yescrypt_kdf32_body(yctx,
+	passwd, passwdlen,
+	flags \| YESCRYPT_PREHASH, N >> 6, 0,
+	dk32);
	if (retval)
	return retval;
-	passwd = dk;
-	passwdlen = sizeof(dk);
+	passwd = dk32;
+	passwdlen = sizeof(dk32);
	}

-	retval = yescrypt_kdf_body(local,
-	passwd, passwdlen, salt, saltlen,
-	flags, N, r, p, t, NROM, buf, buflen);
+	retval = yescrypt_kdf32_body(yctx,
+	passwd, passwdlen,
+	flags, N, t, buf32);
	#ifndef SKIP_MEMZERO
-	if (passwd == dk)
-	explicit_bzero(dk, sizeof(dk));
+	if (passwd == dk32)
+	explicit_bzero(dk32, sizeof(dk32));
	#endif
	return retval;
	}

libbb/yescrypt/alg-yescrypt-platform.c+15 -22

@@ -18,7 +18,7 @@
	* SUCH DAMAGE.
	*/

-	static void alloc_region(yescrypt_region_t region, size_t size)
+	static void alloc_region(yescrypt_region_t *region, size_t size)
	{
	size_t base_size = size;
	uint8_t base, aligned;
@@ -27,39 +27,32 @@ static void alloc_region(yescrypt_region_t region, size_t size)
	//(if defined(MAP_HUGETLB) && defined(MAP_HUGE_2MB)) using 2MB pages
	#else /* mmap not available */
	base = aligned = NULL;
-	if (size + 63 < size) {
-	errno = ENOMEM;
-	} else {
-	base = malloc(size + 63);
-	if (base) {
-	aligned = base + 63;
-	aligned -= (uintptr_t)aligned & 63;
-	}
+	if (size + 63 < size)
+	bb_die_memory_exhausted();
+	base = malloc(size + 63);
+	if (base) {
+	aligned = base + 63;
+	aligned -= (uintptr_t)aligned & 63;
	}
	#endif
	region->base = base;
	region->aligned = aligned;
	region->base_size = base ? base_size : 0;
	region->aligned_size = base ? size : 0;
-	return aligned;
	}

-	static inline void init_region(yescrypt_region_t *region)
+	static void free_region(yescrypt_region_t *region)
	{
-	region->base = region->aligned = NULL;
-	region->base_size = region->aligned_size = 0;
-	}
-
-	static int free_region(yescrypt_region_t *region)
-	{
-	if (region->base) {
	#if 0 //def MAP_ANON
+	if (region->base) {
	if (munmap(region->base, region->base_size))
	return -1;
+	}
	#else
-	free(region->base);
+	free(region->base);
	#endif
-	}
-	init_region(region);
-	return 0;
+	region->base = NULL;
+	region->aligned = NULL;
+	region->base_size = 0;
+	region->aligned_size = 0;
	}

libbb/yescrypt/alg-yescrypt.h+38 -49

@@ -27,21 +27,13 @@
	* This file was originally written by Colin Percival as part of the Tarsnap
	* online backup system.
	*/
-
-	/**
-	* Internal type used by the memory allocator. Please do not use it directly.
-	* Use yescrypt_shared_t and yescrypt_local_t as appropriate instead, since
-	* they might differ from each other in a future version.
-	*/
-	typedef struct {
-	void base, aligned;
-	size_t base_size, aligned_size;
-	} yescrypt_region_t;
-
-	/**
-	* Types for shared (ROM) and thread-local (RAM) data structures.
-	*/
-	typedef yescrypt_region_t yescrypt_local_t;
+	#ifdef YESCRYPT_INTERNAL
+	# if 1
+	# define dbg(...) ((void)0)
+	# else
+	# define dbg(...) bb_error_msg(__VA_ARGS__)
+	# endif
+	#endif

	/**
	* Two 64-bit tags placed 48 bytes to the end of a ROM in host byte endianness
@@ -100,6 +92,16 @@ typedef uint32_t yescrypt_flags_t;
	YESCRYPT_ALLOC_ONLY \| YESCRYPT_PREHASH)
	#endif

+	/**
+	* Internal type used by the memory allocator. Please do not use it directly.
+	* Use yescrypt_shared_t and yescrypt_local_t as appropriate instead, since
+	* they might differ from each other in a future version.
+	*/
+	typedef struct {
+	void base, aligned;
+	size_t base_size, aligned_size;
+	} yescrypt_region_t;
+
	/**
	* yescrypt parameters combined into one struct. N, r, p are the same as in
	* classic scrypt, except that the meaning of p changes when YESCRYPT_RW is
@@ -112,6 +114,19 @@ typedef struct {
	uint64_t NROM;
	} yescrypt_params_t;

+	typedef struct {
+	yescrypt_params_t param;
+
+	/* salt in binary form */
+	/* stored here to cut down on the amont of function paramaters */
+	unsigned char salt[64];
+	size_t saltlen;
+
+	/* used by the memory allocator */
+	//yescrypt_region_t shared[1];
+	yescrypt_region_t local[1];
+	} yescrypt_ctx_t;
+
	/* How many chars base-64 encoded bytes require? */
	#define BYTES2CHARS(bytes) ((((bytes) * 8) + 5) / 6)
	/* The /etc/passwd-style hash is "<prefix>$<hash><NUL>" */
@@ -125,28 +140,6 @@ typedef struct {
	#define HASH_SIZE 32
	#define HASH_LEN BYTES2CHARS(HASH_SIZE)

-	/**
-	* yescrypt_init_local(local):
-	* Initialize the thread-local (RAM) data structure. Actual memory allocation
-	* is currently fully postponed until a call to yescrypt_kdf() or yescrypt_r().
-	*
-	* Return 0 on success; or -1 on error.
-	*
-	* MT-safe as long as local is local to the thread.
-	*/
-	extern int yescrypt_init_local(yescrypt_local_t *local);
-
-	/**
-	* yescrypt_free_local(local):
-	* Free memory that may have been allocated for an initialized thread-local
-	* (RAM) data structure.
-	*
-	* Return 0 on success; or -1 on error.
-	*
-	* MT-safe as long as local is local to the thread.
-	*/
-	extern int yescrypt_free_local(yescrypt_local_t *local);
-
	/**
	* yescrypt_kdf(shared, local, passwd, passwdlen, salt, saltlen, params,
	* buf, buflen):
@@ -180,17 +173,13 @@ extern int yescrypt_free_local(yescrypt_local_t *local);
	* and shared->aligned_size fields may optionally be set by the caller directly
	* (e.g., to a mapped SysV shm segment), without using yescrypt_init_shared().
	*
-	* local must be initialized with yescrypt_init_local().
-	*
	* MT-safe as long as local and buf are local to the thread.
	*/
	#ifdef YESCRYPT_INTERNAL
-	static int yescrypt_kdf(
-	yescrypt_local_t *local,
-	const uint8_t *passwd, size_t passwdlen,
-	const uint8_t *salt, size_t saltlen,
-	const yescrypt_params_t *params,
-	uint8_t *buf, size_t buflen);
+	static int yescrypt_kdf32(
+	yescrypt_ctx_t *yctx,
+	const uint8_t *passwd, size_t passwdlen,
+	uint8_t *buf32);
	#endif

	/**
@@ -209,7 +198,7 @@ static int yescrypt_kdf(
	* MT-safe as long as local and buf are local to the thread.
	*/
	extern uint8_t *yescrypt_r(
-	yescrypt_local_t *local,
-	const uint8_t *passwd, size_t passwdlen,
-	const uint8_t *setting,
-	uint8_t *buf, size_t buflen);
+	const uint8_t *passwd, size_t passwdlen,
+	const uint8_t *setting,
+	uint8_t *buf, size_t buflen
+	);