View Issue Details

IDProjectCategoryView StatusLast Update
0000025file[All Projects] Generalpublic2018-08-11 12:18
ReportercbiedlAssigned Tochristos 
PrioritynormalSeverityminorReproducibilityhave not tried
Status resolvedResolutionfixed 
Product Version 
Target VersionFixed in Version5.35 
Summary0000025: Magic strength computation underflow
DescriptionThis doesn't look good:

    $ file --list | grep FORTRAN
    Strength = 18446744073709551613@7: FORTRAN program text [text/x-fortran]

The problem is in `apprentice_magic_strength`: The `val` variable is size_t but as result of substraction the computation might underflow.

The obvious approach was to guard `val` before such an operation but it turns out some rule start with `!:strength - 1` and the like, so that change would cause other and surprising damage.

Suggestion: Make `val` a signed long which should be big enough to avoid overflow. If the resulting value is still negative, set it to one. Also move that operation a block down to a place where it seems more appropriate.

This yields (as the only change)

    $ file --list | grep FORTRAN
    Strength = 2@7: FORTRAN program text [text/x-fortran]

which looks much better.

As a side effect, the strength of

    @42: Perl5 module source text

went down by one, so adjust this to maintain the current status which is the result of delicate tuning IIRC.
TagsNo tags attached.
Attach Tags

Activities

cbiedl

2018-08-06 06:19

reporter  

strength-underflow.patch (1,546 bytes)
commit 47b0e52009f9bcf91ac608efd2965d163c31b515
Author: Christoph Biedl <debian.axhn@manchmal.in-ulm.de>
Date:   Mon Aug 6 00:52:51 2018 +0200

    src/apprentice.c 2018-08-05-225251 checkpoint _M <7b807237> (+45-44)

diff --git a/magic/Magdir/perl b/magic/Magdir/perl
index 3423b5c3..49572972 100644
--- a/magic/Magdir/perl
+++ b/magic/Magdir/perl
@@ -40,7 +40,7 @@
 0	search/8192	!p
 >0	regex		\^package[\ \t]+[0-9A-Za-z_:]+\ *;
 >>0	regex		\^1\ *;|\^(use|sub|my)\ .*[(;{=]	Perl5 module source text
-!:strength + 75
+!:strength + 76
 
 # Perl POD documents
 # From: Tom Hukins <tom@eborcom.com>
diff --git a/src/apprentice.c b/src/apprentice.c
index 45cf2a9f..c8697159 100644
--- a/src/apprentice.c
+++ b/src/apprentice.c
@@ -841,7 +841,8 @@ private size_t
 apprentice_magic_strength(const struct magic *m)
 {
 #define MULT 10U
-	size_t ts, v, val = 2 * MULT;	/* baseline strength */
+	size_t ts, v;
+	long val = 2 * MULT;	/* baseline strength */
 
 	switch (m->type) {
 	case FILE_DEFAULT:	/* make sure this sorts last */
@@ -947,9 +948,6 @@ apprentice_magic_strength(const struct magic *m)
 		abort();
 	}
 
-	if (val == 0)	/* ensure we only return 0 for FILE_DEFAULT */
-		val = 1;
-
 	switch (m->factor_op) {
 	case FILE_FACTOR_OP_NONE:
 		break;
@@ -969,6 +967,9 @@ apprentice_magic_strength(const struct magic *m)
 		abort();
 	}
 
+	if (val <= 0)	/* ensure we only return 0 for FILE_DEFAULT */
+		val = 1;
+
 	/*
 	 * Magic entries with no description get a bonus because they depend
 	 * on subsequent magic entries to print something.
strength-underflow.patch (1,546 bytes)

christos

2018-08-11 12:18

manager   ~0000049

Applied, but with long -> ssize_t

Issue History

Date Modified Username Field Change
2018-08-06 06:19 cbiedl New Issue
2018-08-06 06:19 cbiedl File Added: strength-underflow.patch
2018-08-11 12:18 christos Assigned To => christos
2018-08-11 12:18 christos Status new => assigned
2018-08-11 12:18 christos Status assigned => resolved
2018-08-11 12:18 christos Resolution open => fixed
2018-08-11 12:18 christos Fixed in Version => 5.35
2018-08-11 12:18 christos Note Added: 0000049