Skip to content

fix(csrf): safely validate decoded token table and fields - #14025

Open
bhuvan-somisetty wants to merge 2 commits into
apache:masterfrom
bhuvan-somisetty:fix-csrf-token-decode
Open

bhuvan-somisetty wants to merge 2 commits into
apache:masterfrom
bhuvan-somisetty:fix-csrf-token-decode

Conversation

@bhuvan-somisetty

Copy link
Copy Markdown

Description

In apisix/plugins/csrf.lua, check_csrf_token decodes the base64 token into JSON without verifying that the result is a table. When a client supplies a token decoding to a JSON primitive (e.g. number, boolean, null), indexing fields like token_table["random"] raises an unhandled Lua runtime error (e.g. attempt to index a number value), causing an HTTP 500 error instead of cleanly rejecting the request.

This PR:

  • Verifies type(token_table) == "table" after JSON decoding.
  • Validates that random, expires, and sign are present with valid scalar types.
  • Adds comprehensive unit test cases in t/plugin/csrf.t verifying that non-table JSON tokens and invalid fields are rejected with HTTP 401 without crashing.

Which issue(s) this PR fixes:

Fixes #14024

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

In `apisix/plugins/csrf.lua`, `check_csrf_token` decodes the base64 token
into JSON without verifying that the result is a table. When a client
supplies a token decoding to a JSON primitive (e.g. number, boolean, null),
indexing fields like `token_table["random"]` raises an unhandled Lua runtime
error (e.g. `attempt to index a number value`), causing a 500 error.

This commit verifies `type(token_table) == "table"` and checks that `random`,
`expires`, and `sign` have valid types, safely returning `false` on invalid
tokens.

Signed-off-by: bhuvan-somisetty <somisettybhuvan5@gmail.com>
Comment thread t/plugin/csrf.t
Comment on lines +424 to +435
=== TEST 18: csrf token with invalid non-numeric expires
--- request
POST /hello
--- more_headers
apisix-csrf-token: eyJyYW5kb20iOjAuMSwiZXhwaXJlcyI6ImludmFsaWQiLCJzaWduIjoiYWJjIn0=
Cookie: apisix-csrf-token=eyJyYW5kb20iOjAuMSwiZXhwaXJlcyI6ImludmFsaWQiLCJzaWduIjoiYWJjIn0=
--- error_code: 401
--- error_log: no expires in token
--- no_error_log
attempt to perform arithmetic
--- response_body
{"error_msg":"Failed to verify the csrf token signature"}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field guards lack direct test coverage

The tests cover primitive tokens and invalid expires, but not JSON null or invalid random and sign values. Add those cases to exercise every new guard.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added tests for null tokens and bad random/sign types. Thanks!

Signed-off-by: bhuvan-somisetty <somisettybhuvan5@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: csrf plugin crashes with runtime error on malformed or non-table base64 token

2 participants