[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#7155: [md5sum] does not accept
bug#7155: [md5sum] does not accept
Wed, 14 Sep 2011 13:35:13 +0100
Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20110707 Thunderbird/5.0
severity 7155 wishlist
tags 7155 + notabug
On 10/16/2010 11:37 PM, Pádraig Brady wrote:
> On 16/10/10 20:37, Rimas Kudelis wrote:
>> On Sun, 03 Oct 2010 23:27:24 +0100, Pádraig Brady <address@hidden>
>>> On 03/10/10 20:24, Rimas Kudelis wrote:
>>>> I have a little problem with md5sum.
>>>> A FreeBSD box generates an md5 sum of a file, which I'm later trying to
>>>> check on a Linux box. The problem is that what FreeBSD's md5 outputs is
>>>> slightly different from what Linux's md5sum expects, which makes md5sum
>>>> complain. The difference is really trivial: md5 outputs one space
>>>> between the sum and the file name, and md5sum outputs/expects two:
>>> md5 seems to output a different format here.
>>> $ head -n1 /etc/motd
>>> FreeBSD 8.0-RELEASE-p3 (GENERIC) #0: Wed May 26 05:45:12 UTC 2010
>>> $ md5sum --version | head -n1
>>> md5sum (GNU coreutils) 8.3
>>> $ md5 file | tee t.md5
>>> MD5 (file) = b85d6fb9ef4260dcf1ce0a1b0bff80d3
>>> $ md5sum -c t.md5
>>> file: OK
>>> Could you verify what md5 utility you're using exactly.
>> Sorry for taking so long to answer, but I wasn't the person producing the
>> checksum, so I had to ask too. The command used to produce the checksum is:
>> $ md5 -r <filename>
>> FreeBSD release version is the same as yours. I've just tested the same
>> command with FreeBSD 6.2, and it only outputs one space too.
> Ah right, md5 -r produces the alternate format.
> I suppose we could support a single space,
> by trying to open("*abc") after trying to open ("abc").
> There is still an ambiguity if both files are present,
> though that is unlikely. I'll have a look.
Thinking more about this, there is a bit of a
security issue with mixing both formats.
Consider the case currently with a checksum in BSD format
of ' important_file' (with leading space).
Now an attacker does:
mv ' important_file' important_file
cp trojan ' important_file'
And since coreutils will check 'important_file' first,
then we'll not report any errors.
If coreutils supported both formats then this would be compounded.
I suppose we could mitigate that somewhat by detecting and
supporting only a single format per run, as done in the following diff.
Note we don't handle the case where the first or only
entry in a BSD format checksum file has a file name with a
leading ' ' or '*'. We could support this and avoid detection
with an option to specify BSD format checksums, but
I don't think that's warranted. Note we could detect
in this situation too by retrying the open with the
leading char included, but that would introduce a security
issue. Consider the following standard format checksum file:
Attackers could then do this undetected:
mv firewall_rules ' firewall_rules'
diff --git a/src/md5sum.c b/src/md5sum.c
index ff9538a..9de34a5 100644
@@ -99,7 +99,7 @@
not include any newline character at the end of a line. */
#define MIN_DIGEST_LINE_LENGTH \
(DIGEST_HEX_BYTES /* length of hexadecimal message digest */ \
- + 2 /* blank and binary indicator */ \
+ + 1 /* blank */ \
+ 1 /* minimum filename length */ )
/* True if any of the files read were the standard input. */
@@ -126,6 +126,9 @@ static bool quiet = false;
improperly formatted. */
static bool strict = false;
+/* Whether a BSD reversed format checksum is detected. */
+static int bsd_reversed = -1;
/* For long options that have no equivalent short option, use a
non-character as a pseudo short option, starting with CHAR_MAX + 1. */
@@ -307,9 +310,24 @@ split_3 (char *s, size_t s_len,
s[i++] = '\0';
- if (s[i] != ' ' && s[i] != '*')
- return false;
- *binary = (s[i++] == '*');
+ /* If "bsd reversed" format detected. */
+ if ((s_len - i == 1) || (s[i] != ' ' && s[i] != '*'))
+ /* Don't allow mixing bsd and standard formats,
+ to minimize security issues with attackers
+ renaming files with leading spaces.
+ This assumes that with bsd format checksums
+ that the first file name does not have
+ a leading ' ' or '*'. */
+ if (bsd_reversed == 0)
+ return false;
+ bsd_reversed = 1;
+ else if (bsd_reversed != 1)
+ bsd_reversed = 0;
+ *binary = (s[i++] == '*');
/* All characters between the type indicator and end of line are
significant -- that includes leading and trailing white space. */
- bug#7155: [md5sum] does not accept,
Pádraig Brady <=