75758c736
Denys Vlasenko
2025-07-07
Subject
libbb/yescrypt: accept longer salts (up to 84 chars)
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)
Binary
No binary (build failed)
Architecture & Compiler Flags
—
Build Log
(log yok)
Commit Diff GitHub →
include/libbb.h +3 -2libbb/yescrypt/alg-yescrypt-common.c +74 -2libbb/yescrypt/alg-yescrypt.h +5 -0testsuite/cryptpw.tests +4 -0
include/libbb.h+3 -2
| @@ -1819,9 +1819,10 @@ extern int crypt_make_rand64encoded(char *p, int cnt /*, int rnd*/) FAST_FUNC; | |
| * "$6$<sha_salt_16_chars><NUL>" | |
| * #define MAX_PW_SALT_LEN (3 + 16 + 1) | |
| * yescrypt: | |
| - | * "$y$j9T$<yescrypt_salt_24_chars><NUL>" |
| + | * "$y$" <up to 8 params of up to 6 chars each> "$" <up to 84 chars salt><NUL> |
| + | * (84 chars are ascii64-encoded 64 binary bytes) |
| */ | |
| - | #define MAX_PW_SALT_LEN (7 + 24 + 1) |
| + | #define MAX_PW_SALT_LEN (3 + 8*6 + 1 + 84 + 1) |
| extern char* crypt_make_pw_salt(char p[MAX_PW_SALT_LEN], const char *algo) FAST_FUNC; | |
| /* Returns number of lines changed, or -1 on error */ | |
libbb/yescrypt/alg-yescrypt-common.c+74 -2
| @@ -63,6 +63,75 @@ fail: | |
| return NULL; | |
| } | |
| + | #if 1 |
| + | static const uint8_t *decode64( |
| + | uint8_t *dst, size_t *dstlen, |
| + | const uint8_t *src, size_t srclen) |
| + | { |
| + | size_t dstpos = 0; |
| + | |
| + | dbg_dec64("src:'%s' len:%d", src, (int)srclen); |
| + | for (;;) { |
| + | uint32_t c, value = 0; |
| + | int bits = 0; |
| + | while (srclen != 0) { |
| + | srclen--; |
| + | c = a2i64(*src); |
| + | if (c > 63) { /* bad ascii64 char, stop decoding at it */ |
| + | srclen = 0; |
| + | break; |
| + | } |
| + | src++; |
| + | value |= c << bits; |
| + | bits += 6; |
| + | if (bits == 24) /* got 4 chars */ |
| + | goto store; |
| + | } |
| + | /* we read entire src, or met a non-ascii64 char (such as "$") */ |
| + | if (bits == 0) |
| + | break; |
| + | /* else: we got last, partial bit block - store it */ |
| + | store: |
| + | dbg_dec64(" storing bits:%d v:%08x", bits, (int)SWAP_BE32(value)); //BE to see lsb first |
| + | while (dstpos < *dstlen) { |
| + | if (srclen == 0 && value == 0) { |
| + | /* Example: mkpasswd PWD '$y$j9T$123': |
| + | * the "123" is bits:18 value:03,51,00 |
| + | * is considered to be 2 bytes, not 3! |
| + | * |
| + | * '$y$j9T$zzz' in upstream fails outright (3rd byte isn't zero). |
| + | * IOW: for upstream, validity of salt depends on VALUE, |
| + | * not just size of salt. Which is a bug. |
| + | * The '$y$j9T$zzz.' salt is the same |
| + | * (it adds 6 zero msbits) but upstream works with it, |
| + | * thus '$y$j9T$zzz' should work too and give the same result. |
| + | */ |
| + | goto end; |
| + | } |
| + | dstpos++; |
| + | *dst++ = value; |
| + | value >>= 8; |
| + | bits -= 8; |
| + | if (bits <= 0) /* can get negative, if we e.g. had 6 bits */ |
| + | goto next; |
| + | } |
| + | dbg_dec64(" ERR: bits:%d dst[] is too small", bits); |
| + | goto fail; |
| + | next: |
| + | if (srclen == 0) |
| + | break; |
| + | } |
| + | end: |
| + | /* here, srclen is 0, no need to check */ |
| + | *dstlen = dstpos; |
| + | dbg_dec64("dec64: OK, dst[%d]", (int)dstpos); |
| + | return src; |
| + | fail: |
| + | *dstlen = 0; |
| + | return NULL; |
| + | } |
| + | #else |
| + | /* Buggy (and larger) original code */ |
| static const uint8_t *decode64( | |
| uint8_t *dst, size_t *dstlen, | |
| const uint8_t *src, size_t srclen) | |
| @@ -87,6 +156,7 @@ static const uint8_t *decode64( | |
| break; | |
| if (bits < 12) /* must have at least one full byte */ | |
| goto fail; | |
| + | dbg_dec64(" storing bits:%d v:%08x", (int)bits, (int)SWAP_BE32(value)); //BE to see lsb first |
| while (dstpos++ < *dstlen) { | |
| *dst++ = value; | |
| value >>= 8; | |
| @@ -104,12 +174,14 @@ static const uint8_t *decode64( | |
| if (!srclen && dstpos <= *dstlen) { | |
| *dstlen = dstpos; | |
| + | dbg_dec64("dec64: OK, dst[%d]", (int)dstpos); |
| return src; | |
| } | |
| fail: | |
| - | *dstlen = 0; |
| + | /* *dstlen = 0; - not needed, caller detects error by seeing NULL */ |
| return NULL; | |
| } | |
| + | #endif |
| static char *encode64( | |
| char *dst, size_t dstlen, | |
| @@ -189,7 +261,7 @@ char *yescrypt_r( | |
| goto fail; | |
| if (*src != '$') { | |
| src = decode64_uint32(&u32, src, 1); | |
| - | dbg("yescrypt has extended params:0x%x", (unsigned)have); |
| + | dbg("yescrypt has extended params:0x%x", (unsigned)u32); |
| if (u32 & 1) | |
| src = decode64_uint32(&yctx->param.p, src, 2); | |
| if (u32 & 2) | |
libbb/yescrypt/alg-yescrypt.h+5 -0
| @@ -33,6 +33,11 @@ | |
| # else | |
| # define dbg(...) bb_error_msg(__VA_ARGS__) | |
| # endif | |
| + | # if 1 |
| + | # define dbg_dec64(...) ((void)0) |
| + | # else |
| + | # define dbg_dec64(...) bb_error_msg(__VA_ARGS__) |
| + | # endif |
| #endif | |
| /** | |
testsuite/cryptpw.tests+4 -0
| @@ -73,6 +73,10 @@ testing 'cryptpw yescrypt with 3-char salt' \ | |
| 'cryptpw -m yescrypt qweRTY123@-+ j9T\$123' \ | |
| '$y$j9T$123$A34DMIGUbUIo3bjx66Wtk2IFoREMIw6d49it25KQh2D\n' \ | |
| '' '' | |
| + | testing 'cryptpw yescrypt with 84-char salt (max size)' \ |
| + | 'cryptpw -m yescrypt qweRTY123@-+ j9T\$123456789012345678901234567890123456789012345678901234567890123456789012345678901234' \ |
| + | '$y$j9T$123456789012345678901234567890123456789012345678901234567890123456789012345678901234$ubrUuPCpI97LIMlVMt/A0Mhs/kBK2UBJYcQSxEZSlz4\n' \ |
| + | '' '' |
| testing 'cryptpw yescrypt implicit' \ | |
| 'cryptpw qweRTY123@-+ \$y\$j9T\$123456789012345678901234' \ | |
| '$y$j9T$123456789012345678901234$AKxw5OX/T4jD.v./IW.5tE/j7izNjw06fg3OvH1LsN9\n' \ | |