Amplify-cli: @auth can be bypassed with list operation
Describe the bug
I can bypass @auth read rules by using the list query instead of get.
This is something that happened "live" so I had to redact some data. I verified the bug using AWS Appsync and DynamoDB from the console. Adding projectMembers via DynamoDB and quering via AWS Appsync.
Amplify CLI Version
4.25.0
To Reproduce
I got this schema
type Project @model
@auth(rules: [
{ allow: groups, groups: ["Admin"]}
{ allow: owner, ownerField: "projectMembers", operations: [read] }
])
{
id: String!
customerId: String!
customerReference: String
name: String!
projectMembers: [String] @auth(rules: [
{allow: groups, groups: ["Admin"]},
{allow: owner, ownerField: "projectMembers", operations: [read, update]}])
}
I have some projects but no projectMembers exists / are empty. I use listProjects and as expected I get nothing.
{
"data": {
"listProjects": {
"items": []
}
}
}
But when I add my user to projectMembers on project 100542867 I can suddenly do this:
I am using getProject to show you which id I added the projectMembers. All other projects should be forbidden.
{
"data": {
"getProject": {
"id": "100542867"
},
"listProjects": {
"items": [
{
"id": "100542867"
},
{
"id": "100950360"
},
{
"id": "100619144"
},
{
"id": "100807769"
},
{
"id": "100513315"
}
]
}
}
}
If I try to get any of those Ids (100950360 in this case) this happens:
{
"data": {
"getProject": null,
"listProjects": {
"items": [
{
"id": "100542867"
},
{
"id": "100950360"
},
{
"id": "100619144"
},
{
"id": "100807769"
},
{
"id": "100513315"
}
]
}
},
"errors": [
{
"path": [
"getProject"
],
"data": null,
"errorType": "Unauthorized",
"errorInfo": null,
"locations": [
{
"line": 2,
"column": 5,
"sourceName": null
}
],
"message": "Not Authorized to access getProject on type Query"
}
]
}
Interestingly enough projectMembers is the only field where the protection works for list
{
"data": {
"listProjects": {
"items": [
{
"id": "100542867",
"projectMembers": [
"2db07842-558a-4ed3-816c-6366cea4e2c4"
]
},
{
"id": "100950360",
"projectMembers": null
},
{
"id": "100619144",
"projectMembers": null
},
{
"id": "100807769",
"projectMembers": null
},
{
"id": "100513315",
"projectMembers": null
}
]
}
},
"errors": [
{
"path": [
"listProjects",
"items",
1,
"projectMembers"
],
"data": null,
"errorType": "Unauthorized",
"errorInfo": null,
"locations": [
{
"line": 5,
"column": 7,
"sourceName": null
}
],
"message": "Not Authorized to access projectMembers on type String"
},
{
"path": [
"listProjects",
"items",
2,
"projectMembers"
],
"data": null,
"errorType": "Unauthorized",
"errorInfo": null,
"locations": [
{
"line": 5,
"column": 7,
"sourceName": null
}
],
"message": "Not Authorized to access projectMembers on type String"
},
{
"path": [
"listProjects",
"items",
3,
"projectMembers"
],
"data": null,
"errorType": "Unauthorized",
"errorInfo": null,
"locations": [
{
"line": 5,
"column": 7,
"sourceName": null
}
],
"message": "Not Authorized to access projectMembers on type String"
},
{
"path": [
"listProjects",
"items",
4,
"projectMembers"
],
"data": null,
"errorType": "Unauthorized",
"errorInfo": null,
"locations": [
{
"line": 5,
"column": 7,
"sourceName": null
}
],
"message": "Not Authorized to access projectMembers on type String"
}
]
}
I could query all the other fields and would get the value but have to redact the data there...
PS: For some reason I only get these 5 or so other projects (seem to always be correlated to which projectId I am added to so maybe something with the order they are in the Database)
Expected behavior
I expect the list to only be of the projects which I have [read] rights on.
Zanndorin
All 24 comments
I get my application to work as intended when switching from
const myProjects = await API.graphql(graphqlOperation(listProjects, {limit: 10}));
to
const myProjects = await API.graphql({
query: listProjects
})
and Appsync somehow fixed itself because it seems to work now aswell..?
Zanndorin
on 5 Aug 2020
Hi @Zanndorin
I followed the exact same steps as you mentioned and I am not able to reproduce it.
These are list of projects I added without project Members
{
"data": {
"listProjects": {
"items": [
{
"id": "testing",
"customerId": "testing"
},
{
"id": "testing1",
"customerId": "testing1"
}
]
}
}
}
I used listProjects command as project member and it returns empty as you mentioned above
{
"data": {
"listProjects": {
"items": []
}
}
}
I added my user to projectMembers on project testing and ran get and list queries:
{
"data": {
"listProjects": {
"items": [
{
"id": "testing",
"customerId": "testing",
"projectMembers": [
"test"
]
}
]
},
"getProject": {
"id": "testing",
"projectMembers": [
"test"
]
}
}
}
As you can see here I am not getting testing1 project in the get and list queries as it doesnt have projectMembers field.
Let me know if I am missing anything here to repro this or there are anymore changes you did on your side ?
akshbhu
on 6 Aug 2020
We are a bit understaffed this week, I'll see if I can find time to triage this a bit more.
How did you query it? Appsync seems to be a bit on and off if it happens.
Quering with the first option gives the issues second one doesn't.
const myProjects = await API.graphql(graphqlOperation(listProjects, {limit: 10}));
const myProjects = await API.graphql({
query: listProjects
})
I also failed to replicate this. Could you post your Query.listProjects and Project.projectMembers templates found in AppSync?
RossWilliams
on 7 Aug 2020
I tried to replicate this in a clean project but I am failing in Appsync with the reverse issue (always protected and projectMembers field is ignored).
But when going back to my production project running Frontend on Localhost:
const myProjects = await API.graphql(graphqlOperation(listProjects, {limit: 10}));
gives me 0 items. (Expected: 1)
Setting limit to 100
gives me 1 item. (Expected: 1)
Setting limit to 1000
gives me 31 (Expected: 1)
const myProjects = await API.graphql({
query: listProjects
})
gives me 1 item. (Previously: 1, Expected: 1)
Adding another project to my user (same query) gives me 1 item. (Previously: 1, Expected: 2)
Running this in Appsync
query MyQuery {
getProject(id: "100486315") {
id
},
listProjects{
items{
id
}
}
}
Gives me
```{
"data": {
"getProject": {
"id": "100486315"
},
"listProjects": {
"items": [
{
"id": "100542867"
}
]
}
}
}
(100542867 being the old project, 100486315 the new one).
Going back to the old query
`const myProjects = await API.graphql(graphqlOperation(listProjects, {limit: 100}));`
again
gives me 1 item. (Expected: 2)
`const myProjects = await API.graphql(graphqlOperation(listProjects, {limit: 1000}));`
gives me 33 items (Expected: 2)
Running
const myProjects = await API.graphql({
query: listProjects
})
```
gives me 1 item (Expected: 2)
There are 2209 items in the DynamoDB.
PS: The protected field is still always protected correctly. i.e. I get projectMembers only for the projects I am a projectMember in.
Zanndorin
on 10 Aug 2020
@RossWilliams
type Project {
id: String!
customerId: String!
customerReference: String
name: String!
projectMembers: [String]
createdAt: AWSDateTime!
updatedAt: AWSDateTime!
}
listProjects(filter: ModelProjectFilterInput, limit: Int, nextToken: String): ModelProjectConnection
?
Zanndorin
on 10 Aug 2020
I assume this has to do with how Filter and Auth are working. If I recall correctly list runs a getAll and filters afterwards?
Sidenote:
project 100542867 is the 9th project that shows up when you open DynamoDB while the other one is number 300++
Zanndorin
on 10 Aug 2020
I also tried it with a while (nextToken) operation
const myProjects = await API.graphql({
query: listProjects
})
setMyProjects(myProjects.data.listProjects.items);
let nextToken = myProjects.data.listProjects.nextToken
while (nextToken) {
let d = await API.graphql({
query: listProjects,
variables: {nextToken}
})
nextToken = d.data.listProjects.nextToken
console.log(d.data)
}
Here, in addition to the expected result, I also get a lot of projects which I should not get. These items do not contain the projectMembers field. Adding the projectMembers field (just an empty list []) to any of these unexpected results seems to resolve the issue for all of them.
Zanndorin
on 10 Aug 2020
It鈥檚 the resolver templates I need to see, not the schema.
RossWilliams
on 10 Aug 2020
@RossWilliams I assumed so.. 馃槉
I do not know where to find them in Appsync so I am providing the ones from my source code ( I assume they are the same?)
These should be fully auto generated.
## [Start] Determine request authentication mode **
#if( $util.isNullOrEmpty($authMode) && !$util.isNull($ctx.identity) && !$util.isNull($ctx.identity.sub) && !$util.isNull($ctx.identity.issuer) && !$util.isNull($ctx.identity.username) && !$util.isNull($ctx.identity.claims) && !$util.isNull($ctx.identity.sourceIp) && !$util.isNull($ctx.identity.defaultAuthStrategy) )
#set( $authMode = "userPools" )
#end
## [End] Determine request authentication mode **
## [Start] Check authMode and execute owner/group checks **
#if( $authMode == "userPools" )
## [Start] Static Group Authorization Checks **
#set($isStaticGroupAuthorized = $util.defaultIfNull(
$isStaticGroupAuthorized, false))
## Authorization rule: { allow: groups, groups: ["Admin"], groupClaim: "cognito:groups" } **
#set( $userGroups = $util.defaultIfNull($ctx.identity.claims.get("cognito:groups"), []) )
#set( $allowedGroups = ["Admin"] )
#foreach( $userGroup in $userGroups )
#if( $allowedGroups.contains($userGroup) )
#set( $isStaticGroupAuthorized = true )
#break
#end
#end
## [End] Static Group Authorization Checks **
## No Dynamic Group Authorization Rules **
## [Start] Owner Authorization Checks **
#set( $isOwnerAuthorized = $util.defaultIfNull($isOwnerAuthorized, false) )
## Authorization rule: { allow: owner, ownerField: "projectMembers", identityClaim: "cognito:username" } **
#set( $allowedOwners0 = $ctx.source.projectMembers )
#set( $identityValue = $util.defaultIfNull($ctx.identity.claims.get("username"), $util.defaultIfNull($ctx.identity.claims.get("cognito:username"), "___xamznone____")) )
#if( $util.isList($allowedOwners0) )
#foreach( $allowedOwner in $allowedOwners0 )
#if( $allowedOwner == $identityValue )
#set( $isOwnerAuthorized = true )
#end
#end
#end
#if( $util.isString($allowedOwners0) )
#if( $allowedOwners0 == $identityValue )
#set( $isOwnerAuthorized = true )
#end
#end
## [End] Owner Authorization Checks **
## [Start] Throw if unauthorized **
#if( !($isStaticGroupAuthorized == true || $isDynamicGroupAuthorized == true || $isOwnerAuthorized == true) )
$util.unauthorized()
#end
## [End] Throw if unauthorized **
#end
## [End] Check authMode and execute owner/group checks **
{
"version": "2018-05-29",
"payload": {}
}
## [Start] Checking for allowed operations which can return this field **
#set( $operation = $util.defaultIfNull($context.source.operation, "null") )
#if( $operation == "Mutation" )
$util.toJson(null)
#else
$util.toJson($context.source.projectMembers)
#end
## [End] Checking for allowed operations which can return this field **
#set( $limit = $util.defaultIfNull($context.args.limit, 100) )
#set( $ListRequest = {
"version": "2018-05-29",
"limit": $limit
} )
#if( $context.args.nextToken )
#set( $ListRequest.nextToken = $context.args.nextToken )
#end
#if( $context.args.filter )
#set( $ListRequest.filter = $util.parseJson("$util.transform.toDynamoDBFilterExpression($ctx.args.filter)") )
#end
#if( !$util.isNull($modelQueryExpression)
&& !$util.isNullOrEmpty($modelQueryExpression.expression) )
$util.qr($ListRequest.put("operation", "Query"))
$util.qr($ListRequest.put("query", $modelQueryExpression))
#if( !$util.isNull($ctx.args.sortDirection) && $ctx.args.sortDirection == "DESC" )
#set( $ListRequest.scanIndexForward = false )
#else
#set( $ListRequest.scanIndexForward = true )
#end
#else
$util.qr($ListRequest.put("operation", "Scan"))
#end
$util.toJson($ListRequest)
## [Start] Determine request authentication mode **
#if( $util.isNullOrEmpty($authMode) && !$util.isNull($ctx.identity) && !$util.isNull($ctx.identity.sub) && !$util.isNull($ctx.identity.issuer) && !$util.isNull($ctx.identity.username) && !$util.isNull($ctx.identity.claims) && !$util.isNull($ctx.identity.sourceIp) && !$util.isNull($ctx.identity.defaultAuthStrategy) )
#set( $authMode = "userPools" )
#end
## [End] Determine request authentication mode **
## [Start] Check authMode and execute owner/group checks **
#if( $authMode == "userPools" )
## [Start] Static Group Authorization Checks **
#set($isStaticGroupAuthorized = $util.defaultIfNull(
$isStaticGroupAuthorized, false))
## Authorization rule: { allow: groups, groups: ["Admin"], groupClaim: "cognito:groups" } **
#set( $userGroups = $util.defaultIfNull($ctx.identity.claims.get("cognito:groups"), []) )
#set( $allowedGroups = ["Admin"] )
#foreach( $userGroup in $userGroups )
#if( $allowedGroups.contains($userGroup) )
#set( $isStaticGroupAuthorized = true )
#break
#end
#end
## [End] Static Group Authorization Checks **
## [Start] If not static group authorized, filter items **
#if( !$isStaticGroupAuthorized )
#set( $items = [] )
#foreach( $item in $ctx.result.items )
## No Dynamic Group Authorization Rules **
## [Start] Owner Authorization Checks **
#set( $isLocalOwnerAuthorized = false )
## Authorization rule: { allow: owner, ownerField: "projectMembers", identityClaim: "cognito:username" } **
#set( $allowedOwners0 = $item.projectMembers )
#set( $identityValue = $util.defaultIfNull($ctx.identity.claims.get("username"), $util.defaultIfNull($ctx.identity.claims.get("cognito:username"), "___xamznone____")) )
#if( $util.isList($allowedOwners0) )
#foreach( $allowedOwner in $allowedOwners0 )
#if( $allowedOwner == $identityValue )
#set( $isLocalOwnerAuthorized = true )
#end
#end
#end
#if( $util.isString($allowedOwners0) )
#if( $allowedOwners0 == $identityValue )
#set( $isLocalOwnerAuthorized = true )
#end
#end
## [End] Owner Authorization Checks **
#if( ($isLocalDynamicGroupAuthorized == true || $isLocalOwnerAuthorized == true) )
$util.qr($items.add($item))
#end
#end
#set( $ctx.result.items = $items )
#end
## [End] If not static group authorized, filter items **
#end
## [End] Check authMode and execute owner/group checks **
#if( $ctx.error )
$util.error($ctx.error.message, $ctx.error.type)
#else
$util.toJson($ctx.result)
#end
These items do not contain the projectMembers field
Thanks, I can reproduce the issue and I believe this is a security issue.
@SwaySway @attilah I'm not very good at velocity, but it appears the issue is that the line
#set( $allowedOwners0 = $item.projectMembers )
nullable is maybe not turned on for AppSync, so we cannot unset $allowedOwners, this results in the previous item in the loop being used. @Zanndorin is able to hit this more often because the auth field is being filtered as well, but this can occur for anyone where a null field follows an allowed auth property.
RossWilliams
on 10 Aug 2020
@RossWilliams do I understand correctly, that your suspicion is that if once in the loop we set a value for $allowedOwners0 because $item.projectMembers has value, then later in the loop is not resetting it to null when $item.projectMembers does no exist on item or it has a null value, so the result is not getting filtered and bypasses auth check by using the previous value?
attilah
on 10 Aug 2020
@attilah Exactly correct. VTL has a configuration option for 'unsetting' properties directive.set.null.allowed. If it isn't set to true, you can't set a value to null.
The PR I submitted has a test that fails without the fix in place, but it depends on the order of the returned values from the Scan operation. If the authorised value is returned last, then the test does not fail, but if the authorised value is 1st or 2nd, the values afterwards are returned as well.
If you notice in the code the dynamic group auth does do a null check, so it is only the owner auth with this issue.
RossWilliams
on 10 Aug 2020
@RossWilliams I will have to check with AppSync team on that, I did not find the VTL settings in AppSync docs documented. Wondering if this is tied to AppSync resolver version and it works when the version is 2017-02-28, since if I recall correctly we changed it recently to 2018-05-29.
Thanks for the PR! To make the fix in the test predictable, would you add a sort key to the model and used that to make sure we've null members in the middle? Meanwhile I try to play with the null setting in VTL.
EDIT: Template version does not matter since that's only affecting DataSource operations: https://docs.aws.amazon.com/appsync/latest/devguide/resolver-mapping-template-changelog.html
attilah
on 10 Aug 2020
@RossWilliams I can confirm that null behavior is what you suspected, since for this resolver template:
#set ($vv = [0,1,2,3,0,4,0,5,6,0])
#set ($items = [])
#set ($match = 0)
#foreach ($v in $vv)
#if ($v == $match)
#set ($tempv = $null)
#else
#set ($tempv = $v)
#end
$util.qr($items.add($tempv))
#end
$util.error($util.toJson($items))
this is the result:
[null,1,2,3,3,4,4,5,6,6]
3,4,6 should not repeat.
attilah
on 10 Aug 2020
test updated with sort key.
RossWilliams
on 10 Aug 2020
@RossWilliams thanks, awesome!
I checked the other resolvers (create and subscription), where we have the same logic and the generated code looks like this:
set(ref(allowedOwnersVariable), raw(`$util.defaultIfNull($${variableToCheck}.${ownerAttribute}, null)`))
And it causes the same problem, since null assignment will not work end of the day.
Would you add that to the fix as well to have it covered in 1 PR.
https://github.com/aws-amplify/amplify-cli/blob/master/packages/graphql-auth-transformer/src/resources.ts#L413
https://github.com/aws-amplify/amplify-cli/blob/master/packages/graphql-auth-transformer/src/resources.ts#L490
attilah
on 10 Aug 2020
@RossWilliams sorry, my bad...I looked at what we are generating with the additional 2 lines I requested and it seems that are actually fine in a sense that those variables are not reused in a loop and on the other hand the first one at line 413 is augmenting the input, which this way disables the automatic owner assignment. Please undo those 2 null value changes, that seems to be the cause of the mock test failures.
EDIT: nvm, reverted the change
attilah
on 10 Aug 2020
@attilah So this is fixed by regenerating the resolver code after updating?
Zanndorin
on 10 Aug 2020
@Zanndorin that's correct. You don't have to wait for the release, just copy the generated list resolver response templates to turn them into custom resolvers into the resolvers folder and apply the fix and deploy.
attilah
on 10 Aug 2020
@attilah
I'm on 4.27.1 now but the listProjects autogenerated resolver is still is the same (& issue still exists). Amplify removed a lot of other resolvers that existed before (which mock api creates again but they are removed when I close down the mock. listProject resolver also looks the same there). I tried running amplify codegen. Not sure if this is a issue where I am the idiot or there is a different issue.
PS: How do I find/edit the resolvers in Appsync?
Zanndorin
on 11 Aug 2020
@Zanndorin Did you try creating an empty line in the schema.graphql file? It's been a useful trick for me in the past, but I'm not sure if it'll work in your case.
houmark
on 11 Aug 2020
@houmark
I'll tried something similar at first and tried yours now.
Amplify push updates all the resolvers
UPDATE_IN_PROGRESS ListProjectResolver AWS::AppSync::Resolver Tue Aug 11 2020
but the code in them seems to stay the same / bug seems to persist.
(Tried running codegen after addig and empty line aswell but no changes)
Zanndorin
on 11 Aug 2020
OK sorry guys I got it to work.
I think all my issues were related to IntelliJ and it not syncing / saving files correctly.
Zanndorin
on 11 Aug 2020
Related issues
nicksmithr
路
3Comments
jexh
路
3Comments
ReidWeb
路
3Comments
darrentarrant
路
3Comments
YikSanChan
路
3Comments