Re: [bug-bash] $RANDOM not Cryptographically secure pseudorandom number generator
Hello there, I've reviewed both patches and found some things that should be either greatly improved, or buried some place very deep. :-p On 2019-01-07 08:15, Ole Tange wrote: On Mon, Jan 7, 2019 at 12:08 AM Chet Ramey wrote: On 1/5/19 3:12 PM, Eduardo A. Bustamante López wrote: > On Fri, Dec 28, 2018 at 10:24:50AM +0100, Ole Tange wrote: > (...) >> Patch attached. What's the period of the resulting RNG? That's the chief complaint with the existing implementation. My implementation made that hard to determine. The Salsa20-based RNG from salsa20.patch is completely broken. Please do not use this earlier patch: * it doesn't follow the Salsa20 specification (specifically, the Salsa20 state layout is completely ignored) * the RNG itself leaks part of the Salsa20 state (15 bits from each of the 16 32-bits integers) as its output, which is then used as input to generate the next set of numbers; this at the very least hugely facilitates full prediction of future values from a restricted sample output of the RNG I have updated the patch: It now supports BASH_RANDOM_16 as well as 32. I have changed the algorithm to ChaCha20, as it seems that is the variant that Google, FreeBSD, OpenBSD, and NetBSD have chosen, and it seems it is a little harder to attack. Now this new ChaCha20-based RNG sports a much better implementation (the state layout is mostly correct, and it does not leak its internal state). A state layout issue remains in that upon reseed, the entire state gets altered, which is bad: the constant in `chachastate` must remain unaltered, otherwise this is no longer ChaCha20 (and beyond that, it is unnecessary to alter both the key and nonce/counter). Also, `stringaddseedrand()` allegedly "shakes" the bits if the seed is longer than the state, by calling `chacha20_block()`, but the only thing this actually does is to increase the counter value in the state (since with the switch to ChaCha20 the block primitive rightfully no longer alters the whole state). However, I've got some much more fundamental concerns with this whole idea. In short, just because there's a top-notch stream cipher in it doesn't mean it's a (good) CSPRNG. And just like with ciphers, if you don't know what you're doing really well, don't roll your own CSPRNG. Specifically, this ChaCha20-based RNG may be computationally secure (see https://crypto.stackexchange.com/a/39194), but it's neither forward secret nor prediction resistant (the Salsa20-based RNG, though much more messy, was however forward secret). Moreover, its computational security isn't even that strong, since it depends on the seed remaining secret. The initial seed being merely a combination of the current time of day and shell PID, it's not random enough and could be brute-forced. All in all, this makes for a pretty weak CSPRNG, one that I would be unwilling to use to generate keys. To improve it, the first step would be to add real entropy (from /dev/urandom) to the mix, then use a vetted design such as HMAC-DRBG or CTR-DRBG (this one *may* play well with ChaCha20). These would give us something computationally secure, forward secret and prediction resistant. Sadly, this would not be trivial to reconcile with the requirement that seeding $RANDOM with a known value gives the same random stream (this conflicts with the prediction resistance property of a CSPRNG). :( In my opinion, $RANDOM doesn't need to change: being deterministic, it's never gonna be a CSPRNG, so the linear congruential RNG is fine. If you really need some quality CSPRNG values, I'd suggest adding a $SECURE_RANDOM variable that just reads from /dev/urandom. Implementing a full CTR-DRBG seems like way too much work for a shell. (And please feel free to clean up my code: C is a language I code once every 5 years or so). (So while I'm at it: `chachastate` keeps switching types and is sometimes a global, sometimes a local variable. That's confusing.) /OIe Cheers, Quentin
Re: [bug-bash] $RANDOM not Cryptographically secure pseudorandom number generator
On January 20, 2019 2:39:45 PM UTC, Martijn Dekker wrote:
>filename_suffix() {
> chars=ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789
>length=${#chars}
>for ((i=0; i<10; i++)) do
>printf '%s' "${chars:$(( SECURE_RANDOM % length + 1 )):1}"
>done
>}
The character distribution here will be biased, because ${#chars} is not a
power-of-2.
TL;DR: discard out-of-range values instead of wrapping them with %:
filename_suffix() {
chars=ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789
pow2length=64
result=
while (( ${#result} < 10 )); do
result+="${chars:$(( SECURE_RANDOM % pow2length )):1}"
done
printf '%s' "$result"
}
$pow2length is the next greater-or-equal power-of-2 starting from ${#chars}. It
is fixed here for brevity, since the array is fixed as well, but could be
computed.
If the value of $(( SECURE_RANDOM % pow2length )) is greater than the length of
$chars, no character is added. This means that the loop may iterate more than
10 times to yield 10 characters.
To elaborate on why % doesn't work, consider this: $RNG generates 3-bits random
values, so in the range 0-7, and I want integers in the range 0-4. If I naively
use %, I get this distribution:
$RNG -> $RNG % 5
0 -> 0
1 -> 1
2 -> 2
3 -> 3
4 -> 4
5 -> 0
6 -> 1
7 -> 2
Clearly I'm less likely to get 3s and 4s. The same logic applies to your use of
% with ${#chars}.
If external tools are not an issue and somewhat wasted resources do not itch
you, for this purpose of generating random strings, dd+tr usually nails it
(brevity-wise at least):
dd if=/dev/urandom bs=1024 count=1 status=none | tr -d -c A-Za-z0-9
Then truncate to whatever length you need, or repeat if more characters are
needed.
Cheers,
Quentin
Re: Bug on function.
On 2015-12-08 08:16, Kelvin Tan Thiam Teck wrote: Hi, Please try my payload on that script, before telling me what $@ and $* does. and see if my param1 injection will cause your system to reboot on 18th param. it has nothing to do with $@ & $*, it's another bugs on bash which i found out, similar to shockbash, except it's harder to execute due to the requirement for it to happen. Regards KT Hi, I tried your payload on that script, using Bash 4.2.37(1)-release. It behaves exactly as expected: when calling the Gateway function, $* expands the script's "$1" to separate words and "reboot" becomes the 9th positional parameter to the function. Then 'echo "9th: `$9`"' tries to execute 'reboot' (and fails since I'm not root, in my case). Note all other "echo"s don't have those backticks. Nothing special happens on param 18 since $18 expands as expected to "$1"8 = "echo8" inside the function (and that command does not exist, at least on my system). Please provide your output to that script (not run as root so as not to make the system reboot) if it's different from what I describe above. Regards, Quentin
Re: Bug on function.
On 2015-12-08 02:45, Kelvin Tan Thiam Teck wrote:
hi, there's a bug on function that allow attacker to inject
parameters.
./report.sh "echo ln -s /sbin/halt; mv halt ;reboot8 ; reboot" AAA AAA
AAA AAA AAA AAA AAA AAA AAA AAA AAA AAA AAA AAA AAA AAA AAA AAA AAA
AAA
#!/bin/bash
function library {
echo ${@}
}
function Gateway {
unset param
param[7]="$8"
piaram[8]="$9"
param[9]="$10"
param[10]="$11"
param[11]="$12"
param[12]="$13"
param[13]="$14"
param[14]="$15"
param[15]="$16"
param[16]="$17"
param[17]="$18"
param[18]="$19"
#echo "After Passing Thru Function: ${param[@]}"
echo "9th: `$9`"
echo "10th: $10"
echo "11th: $11"
echo "12th: $12"
echo "13th: $13"
echo "14th: $14"
echo "15th: $15"
echo "16th: $16"
echo "17th: $17"
$18
echo "19th: $19"
echo "20th: $20"
}
echo "Before Passing Thru Function: $*"
Gateway $*
Hi,
I think you should read the "Special Parameters" and "Parameter
Expansion" sections of the Bash man page.
Specifically:
* $* does not expand parameters as separate words (as "$@" does)
* positional parameters with more than 1 digit require braces: "${11}"
Cheers,
Quentin
Extra '-i' file in Bash repo
Hello there, Commit 43aebe922bc2a614c410e282fdf772e063454168 added an empty file named '-i' at the root of the Bash repo. This might be an elaborate trick to auto-add the -i flag to all commands run with * as argument in the repo directory, or a mistake. Just thought I'd mention it in case it's the latter. Regards, Quentin
stty not restored in trap when executing read with options
Hello, bash: 4.4.12(1)-release stty: 8.25 (GNU version) When running this small script: > trap 'stty echo; exit 0' INT > stty -echo > read -d a and hitting Ctrl-c stty should be restored in the trap, but doesn't (the trap code is executed though). This also seems to happen with read -s and -n options. Calling read without options restores stty as expected. ksh (93u+) and zsh (5.2) also restores stty as expected. Finally tested this on bash 3.2.57(1)-release and it's working as expected, stty is restored in the trap. -- Quentin L'Hours
Re: PDF documentation output not readable (3.2.6 GNU Parallel)
On 2018-05-10 02:24 PM, Greg Wooledge wrote: > Oh... well, it's not in the manual (bash(1)). It's in this > other thing called the "Bash Reference Manual", apparently at > <https://tiswww.case.edu/php/chet/bash/bashref.html#GNU-Parallel> This is the html version of the info documentation if I'm not mistaken, You can see it from your terminal with: info bash 'Basic Shell Features' 'Shell Commands' 'GNU Parallel' -- Quentin
Re: Misbehavior with constants and bash script
Hi Chet, On 2018-11-19 03:38 PM, Chet Ramey wrote: > When the assignment is used as an argument to `declare', it causes the > declare command to fail, but it's not a variable assignment error, so > the script simply continues as with any other failed command. I remembered this email thread about declaration utilities splitting behavior: https://lists.gnu.org/archive/html/help-bash/2018-01/msg6.html If declaration utilities splitting is modified to be similar to a basic assignment then wouldn't it make sense to do the same for the rest (in this case assigment errors)? It feels like POSIX progressively wants declaration builtins to have the same rules as basic assignments, why should assignment error stay different? -- Quentin
[minor] Space after last element on assoc print
Hi, Bash Version: 4.4 Release Status: release Description: Useless space after last item of a declare -p on an assoc array (indexed arrays don't print it, and neither does ksh typeset on assoc arrays). It doesn't seem to have any consequence though. Repeat-By: $ declare -A capital[fr]=Paris $ declare -p capital declare -A capital=([fr]="Paris" ) Fix: Maybe just rlen-- just after looping on all elements in assoc.c -- Quentin L'Hours
Re: [minor] Space after last element on assoc print
On 10/20/2016 10:40 PM, Dan Douglas wrote: On Wed, Oct 19, 2016 at 11:47 PM, Quentin L'Hours Useless space after last item of a declare -p on an assoc array (indexed arrays don't print it, and neither does ksh typeset on assoc arrays). It doesn't seem to have any consequence though. Repeat-By: $ declare -A capital[fr]=Paris $ declare -p capital declare -A capital=([fr]="Paris" ) You cannot assign an attribute to an individual element of any array. The behaviour for indexed arrays is described in the manual: "declare -a name[subscript] is also accepted; the subscript is ignored." In the case of a previously declared associative array in the current scope, one might argue that bash should throw an error instead. I think what you're seeing is bash taking a "reasonable default" instead of throwing an error in response to a nonsensical assignment. I think you misunderstood what I was really talking about, I was just pointing out a useless space printed by declare -p on an assoc array (which isn't present for indexed arrays). I'm not trying to add any attribute to an individual element, I'm just printing the contents of the array. Sorry if the example was misleading, -- Quentin L'Hours
