Lo que te da una auditoría

Este es el informe de una pequeña API de pedidos. Cada hallazgo nombra archivo y línea, cita el código, explica qué gana un atacante y termina con un parche y cómo verificarlo.

Escrito a mano para esta página: el repositorio es ficticio y no se llamó a ningún modelo. Tus informes se escriben en el idioma en el que usas el sitio.

acme/checkout-api

Resumen

The service exposes an order API backed by Postgres and a Stripe webhook. Two issues are exploitable by an unauthenticated or lightly authenticated caller: a SQL query built by string concatenation in the order lookup, and an order endpoint that trusts the identifier in the URL without checking who owns it. A Stripe secret is committed in `src/config/env.ts`, which makes rotation the first task regardless of the rest. Session handling and the webhook signature check are sound.

Puntuación de riesgo
82
Hallazgos
4
  1. 01CríticaSQL injection in the order lookupCWE-89

    src/routes/orders.ts:42–46 · confianza high · fable + codex

    `GET /orders/:id` builds its query by concatenating the `id` path parameter into SQL. The parameter is taken straight from the URL and never validated, so any caller controls the statement that runs.

    const { rows } = await db.query(
      "select * from orders where id = " + req.params.id,
    );

    Impacto

    Full read of the orders table, including customer names, addresses and payment references, and a plausible path to writing or dropping data.

    Escenario de explotación

    A request to `/orders/1 or 1=1` returns every order in the table. `/orders/1; drop table orders --` runs a second statement on a driver that allows it. No account is needed: the route is reachable before the session check on line 39.

    Corrección

    Pass the identifier as a bound parameter and reject anything that is not a UUID before the query runs.

    Cómo corregirlo

    1. Validate the parameter: `if (!isUuid(req.params.id)) return res.status(400).json({ error: "Invalid order id." });`
    2. Replace the concatenated query with a parameterized one, as in the patch below.
    3. Search the rest of `src/routes` for the same pattern — `db.query("… " +` — and convert those too.
    4. Add a regression test that requests `/orders/1 or 1=1` and expects 400.

    Parche

    const { rows } = await db.query(
      "select * from orders where id = $1",
      [req.params.id],
    );

    Cómo verificarlo

    Replay `GET /orders/1%20or%201=1`: it must answer 400, and the Postgres log must show the parameterized statement with the raw value bound, not interpolated.

  2. 02AltaAny signed-in user can read any orderCWE-639

    src/routes/orders.ts:39–41 · confianza high

    The handler checks that a session exists but never compares the order's owner with the caller. The identifier in the URL is the only thing that decides which record is returned.

    if (!req.session?.userId) return res.status(401).end();
    // … no ownership check before the query below

    Impacto

    Cross-customer data exposure: addresses, totals and payment references belonging to other accounts.

    Escenario de explotación

    A customer who signs up legitimately can iterate order identifiers and read other customers' orders. Nothing in the logs distinguishes this from normal use.

    Corrección

    Scope the query to the session's user, so an identifier alone is never enough.

    Cómo corregirlo

    1. Add `and user_id = $2` to the query and pass `req.session.userId`.
    2. Return 404 rather than 403 when the row is missing, so the endpoint does not confirm which identifiers exist.
    3. Apply the same scoping to `PATCH /orders/:id` and `POST /orders/:id/refund`.

    Parche

    const { rows } = await db.query(
      "select * from orders where id = $1 and user_id = $2",
      [req.params.id, req.session.userId],
    );
    if (rows.length === 0) return res.status(404).end();

    Cómo verificarlo

    Sign in as one account and request an order created by another: the response must be 404, and the row must not appear in the body.

  3. 03AltaStripe secret key committed to the repositoryCWE-798

    src/config/env.ts:12 · confianza high · fable + codex

    A live Stripe secret key is assigned as a fallback when the environment variable is missing. Anyone with read access to the repository — or to its history — has the key.

    export const STRIPE_KEY = process.env.STRIPE_SECRET_KEY ?? "sk_live_51H…";

    Impacto

    Full API access to the payment account, independent of this service.

    Escenario de explotación

    The key is in every clone and in the commit history. It allows charges, refunds and customer reads against the live account until it is rotated.

    Corrección

    Rotate the key first, then remove the fallback so a missing variable fails loudly at boot.

    Cómo corregirlo

    1. Roll the key in the Stripe dashboard now; the committed one must be treated as public.
    2. Replace the fallback with a hard failure, as in the patch.
    3. Purge the value from history (`git filter-repo`) or accept that the old key stays exposed and rely on the rotation.
    4. Add a secret scan to CI so the next one is caught before it merges.

    Parche

    const key = process.env.STRIPE_SECRET_KEY;
    if (!key) throw new Error("STRIPE_SECRET_KEY is not set");
    export const STRIPE_KEY = key;

    Cómo verificarlo

    Start the service without `STRIPE_SECRET_KEY`: it must refuse to boot instead of using a built-in value.

  4. 04MediaNo rate limit on the refund endpointCWE-770

    src/routes/orders.ts:88 · confianza medium

    `POST /orders/:id/refund` performs a Stripe call per request with no limit per account or per order. The handler is also not idempotent: two requests that arrive together can both pass the `status !== "refunded"` check.

    router.post("/orders/:id/refund", requireSession, async (req, res) => {

    Impacto

    Double refunds and a bill at the payment provider for traffic nobody asked for.

    Escenario de explotación

    A customer sending the same refund twice within a few milliseconds can have the order refunded twice before the status is written back.

    Corrección

    Limit the endpoint per account and make the refund idempotent at the database level.

    Cómo corregirlo

    1. Add a limiter keyed on the session's user, e.g. 5 refunds per hour.
    2. Claim the refund with a conditional update — `update orders set status = 'refunding' where id = $1 and status = 'paid' returning id` — and only call Stripe when a row comes back.
    3. Pass an idempotency key to the Stripe call so a retry cannot charge twice.

    Cómo verificarlo

    Fire two refund requests for the same order in parallel: exactly one must reach Stripe, and the second must answer 409.

Próximos pasos

  1. Rotate the committed Stripe key.
  2. Parameterize the order lookup and add the ownership check.
  3. Make refunds idempotent and rate limited.
  4. Add a secret scan and a query-building lint rule to CI.

Bien hecho

  • The Stripe webhook verifies the signature against the raw request body before parsing it.
  • Session cookies are `HttpOnly`, `SameSite=Lax` and `Secure` in production.
  • Database credentials come from the environment; only the Stripe key had a committed fallback.
5 archivos analizados
  • src/routes/orders.ts
  • src/db/client.ts
  • src/lib/session.ts
  • src/config/env.ts
  • src/routes/webhooks.ts
Auditar tu repositorioCómo funciona